Changed Error Message if Entity is not configured in dab-config json #2186
Conversation
sourabh1007
requested review from
mbhaskar,
Aniruddh25,
seantleonard,
Mathos1432,
gledis69,
aaronburtle,
tarazou9,
ayush3797,
abhishekkumams,
aaronpowell,
severussundar,
ravishetye,
rohkhann and
neeraj-sharma2592
as code owners
|
/azp run |
|
/azp run |
| } | ||
| else | ||
| { | ||
| throw new ApplicationException($"The entity '{key}' was not found in the dab-config json"); |
There was a problem hiding this comment.
why ApplicationException and not DataApiBuilderException which is used throughout the project?
There was a problem hiding this comment.
It makes sense, Since, it is a small change, making this change in patch PR
There was a problem hiding this comment.
@sajeetharan what do you think?
There was a problem hiding this comment.
also message should be something like:
"... was not found in the runtime config."
using "dab-config json" is too close to "dab-config.json" which is an explicit file name. End-users most likely aren't using dab-config.json as the file name (more likely dab-config.mssql.json, dab-config.cosmos.json etc.) so this may add confusion.
| using (TestServer server = new(Program.CreateWebHostBuilder(args))) | ||
| using (HttpClient client = server.CreateClient()) | ||
| { | ||
| string query = @"{ | ||
| EntityName { | ||
| items{ | ||
| id | ||
| title | ||
| } | ||
| } | ||
| }"; | ||
|
|
||
| object payload = new { query }; | ||
|
|
||
| HttpRequestMessage graphQLRequest = new(HttpMethod.Post, "/graphql") | ||
| { | ||
| Content = JsonContent.Create(payload) | ||
| }; | ||
|
|
||
| ApplicationException ex = await Assert.ThrowsExceptionAsync<ApplicationException>(async () => await client.SendAsync(graphQLRequest)); | ||
| Assert.AreEqual(ex.Message, "The entity 'Planet' was not found in the dab-config json"); |
There was a problem hiding this comment.
why is this test sending a GraphQL request? if the entity doesn't exist in the config, shouldn't there be a startup error?
Alternatively, if the intent is to capture some sort of error that occurs during runtime, then erroring on a non-existent entity should be handled by HotChocolate which detects that the requested entity is not part of the GraphQL schema -> HTTP400 bad request/GraphQL error because request doesn't adhere to schema.
There was a problem hiding this comment.
It is not schema error, it is config error, if there is an entity which is defined in schema but not defined in config then, Hotchocolate wouldn't even know about it since schema has this information. It will fail when DAB check for the entity in the config. Hence, failed during the operation.
There was a problem hiding this comment.
can you add more details to the test description then? there's no context in the test describing what mismatch exists between schema.gql and dab-config.json. The request is querying "EntityName", but the error message says "Planet". That's not really actionable by the end user because it's not immediately clear what the relation is (or isn't) between "EntityName" and "Planet"
|
PR description nor the linked issue describe where this issue was surfacing: as a response to a request, or during DAB startup. Please include a before/after request that shows how your fix appears to the end user. |
… runtime config (#2272) ## Why make this change? Customers are getting` System.Collections.Generic.KeyNotFoundException` when there configuration is missing for an entity defined in schema file. ## What is this change? As part of this PR, throwing proper message saying, entity is not present in runtimeconfig. Similar changes were made at this PR also #2186 ## How was this tested? - [ ] Integration Tests - [ ] Unit Tests Closes #2266 #887 --------- Co-authored-by: Sean Leonard <sean.leonard@microsoft.com>
Why make this change?
As mentioned here #887, if entity is not configured in DAB Config JSON file, it throws a generic exception i.e. keyNotFoundException.
As part of this PR, changed this message to make it clearer.
What is this change?
If Entity is not configured then DAB will throw
System.ApplicationExceptionwith error message as "The entity '' was not found in the dab-config json"How was this tested?
Sample Request(s)