Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Some invalid queries get exposed as "unknown" or "Intenal" instead of "ValidationError" or "BadRequest" in micrometer #1058

Closed
hpuac opened this issue May 19, 2022 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@hpuac
Copy link

hpuac commented May 19, 2022

Expected behavior

When modifying the GraphQL request and sending a variable that is not in the query, I would expect it to get exposed as ValidationError or BAD_REQUEST in micrometer.

Actual behavior

  • When modifying the GraphQL request and sending a variable that is not in the query, it gets exposed as errorType unknown in micrometer.
  • When modifying a federated GraphQL request and sending a variable that is not in the query, it gets exposed as errorTyoe INTERNAL in micrometer.

This is preventing me to exclude client errors from our alerting that we base on these metrics.

Steps to reproduce

I think the behaviour description is pretty self explanatory. Please let me know if something is not clear.
But I'd like to provide some additional information.

  • When the error type "unknown" is exposed, the exception that DgsGraphQLMetricsInstrumentation->instrumentExecutionResult receives is graphql.execution.InputMapDefinesTooManyFieldsException: The variables input contains a field name 'myDemoKey' that is not defined for input object type 'MyDemoInput' . The ErrorUtils.sanitizeErrorPaths() call then sets the type to unknown.
  • When the error type INTERNAL is exposed, the exception that DgsGraphQLMetricsInstrumentation->instrumentExecutionResult receives is TypedGraphQLError{message='java.lang.IllegalStateException: Expected key 'myDemoKey' in received federated values map does not exist.', locations=[], path=[_entities], extensions={errorType=INTERNAL}}. The ErrorUtils.sanitizeErrorPaths() call then sets the type to INTERNAL.
@hpuac hpuac added the bug Something isn't working label May 19, 2022
@berngp
Copy link
Contributor

berngp commented May 20, 2022

Thanks @hpuac, it should be consistent with BAD_REQUEST in both cases.

@berngp berngp self-assigned this May 20, 2022
@srinivasankavitha
Copy link
Contributor

Closing this issue since it should be addressed in the latest release. Please reopen, if issue exists.

@hpuac
Copy link
Author

hpuac commented Jul 5, 2022

Hey @srinivasankavitha, sorry for the delay but I was now able to re-test it with DGS 5.0.5.
The following errors are still not exposed to micrometer as expected:

  • Exposed as INTERNAL: {"message":"com.netflix.graphql.dgs.exceptions.MissingFederatedQueryArgument: The federated query is missing field(s) __typename","locations":[],"path":["_entities"],"extensions":{"errorType" :"INTERNAL"}}
  • Exposed as unknown: {"message":"Variable 'representations' has an invalid value: Variable 'representations' has coerced Null value for NonNull type '[_Any!]!'","locations":[{"line":1,"column":23}],"extensions":{ "classification":"ValidationError"}}
  • Exposed as unknown: {"message":"Variable 'input' has an invalid value: Variable 'input' has coerced Null value for NonNull type 'MyDemoInput!'","locations":[{"line":1,"column":36}],"extensions":{"c lassification":"ValidationError"}}
  • Exposed as unknown: {"message":"The variables input contains a field name 'myDemoName' that is not defined for input object type 'MyDemoInput' ","extensions":{"classification":"ValidationError"}}
  • Does not get exposed to micrometer as error at all: {"message":"The query is null or empty.","locations":[],"extensions":{"errorType":"BAD_REQUEST"}}

@hpuac
Copy link
Author

hpuac commented Jul 7, 2022

Hey @berngp and @srinivasankavitha, should I create a new issue or will you reopen this one? 🙂

@berngp
Copy link
Contributor

berngp commented Jul 7, 2022

Let's keep this one.

@hpuac
Copy link
Author

hpuac commented Jul 7, 2022

@berngp do you have the rights to reopen it? I don't have any button for it.

@berngp berngp reopened this Jul 7, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Sep 29, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Sep 29, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Sep 29, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Oct 6, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Oct 6, 2022
berngp added a commit that referenced this issue Oct 10, 2022
#1058: Correct error types for bad user input in micrometer
antholeole added a commit to antholeole/dgs-framework that referenced this issue Oct 19, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Oct 20, 2022
antholeole added a commit to antholeole/dgs-framework that referenced this issue Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants