-
Notifications
You must be signed in to change notification settings - Fork 320
Controller's with CompletableFuture return bad error messages or no error at all #479
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
Comments
I could not easily reproduce this. There is something less obvious going on. Could you create a sample through https://start.spring.io/? |
Ah, yes it turns out it seems to only happen when graphql-java-extended-validation is added into the runtime wiring:
I can't tell if this is a Spring issue or GraphQLJava? Or if the configuration is wrong for use with Spring, I've just used the standard directive wiring on the github page. |
Looks like FieldValidatorDataFetcher causes the DataFetcher to bypass ContextDataFetcherDecorator in GraphQlMetricsInstrumentation somewhere. Not sure if this relates to changes made in, or is similar to, this issue #240 |
This is where the problem happens in ContextDataFetcherDecorator.java:119 Since FieldValidatorDataFetcher is in the graphql package it doesn't get added as a ContextDataFetcherDecorator |
That does sound like a problem. I'll schedule it to have a closer look. We'll probably have refine how we decide if a |
Thanks for the quick fix, when's the next release cycle for 1.0.2 likely to be? |
Next week. It's set for Sep 20 now. |
Uh oh!
There was an error while loading. Please reload this page.
I can't tell if this is bad API use on my end, as I'm not an expert with Reactor/Async etc.
But my controllers that return a Completable future of a List type e.g
return bad error messages when there are any exceptions outside of the CompletableFuture. In this case, I missed the
@Argument
annotation on the timestamp parameter but only got this generic message:Or if the type is a String it simply just gets passed through to the final query like this:
The text was updated successfully, but these errors were encountered: