graphql-java-kickstart / graphql-java-tools

A schema-first tool for graphql-java inspired by graphql-tools for JS
https://www.graphql-java-kickstart.com/tools/
MIT License
812 stars 174 forks source link

ClassCastException when a resolver is simultaneously suspending & using the custom context class. #318

Open alacoste opened 5 years ago

alacoste commented 5 years ago

Example that would trigger the bug:

graphql schema:

schema {
  query: Query
  mutation: Mutation
  subscription: Subscription
}

type Query {
  myQuery: Boolean!
}

Resolver:

object Query : GraphQLQueryResolver {
  suspend fun myQuery(context: MyCustomContextType): Boolean {
    /* some logic */
    return true
  }
}

With such an example, graphql-java-kickstart successfully wires the schema, but at runtime during resolution of the 'myQuery' field, the following ClassCastException error is thrown:

ERROR ace.graphql.errors.CustomDataFetcherExceptionHandler - class graphql.schema.DataFetchingEnvironmentImpl cannot be cast to class MyCustomContextType

After some debugging I believe that the bug resides in MethodFieldResolver.kt, inside of override fun createDataFetcher(): DataFetcher<*> in the following block of code:

override fun createDataFetcher(): DataFetcher<*> {
        [...]

        // Add DataFetchingEnvironment/Context argument
        if (this.additionalLastArgument) {
            when (this.method.parameterTypes.last()) {
                null -> throw ResolverError("Expected at least one argument but got none, this is most likely a bug with graphql-java-tools")
                options.contextClass -> args.add { environment -> environment.getContext() }
                else -> args.add { environment -> environment }
            }
        }

        [...]
}

This piece of code is the one that creates the lambda that will compute the additional environment/context argument passed to the method at resolution time from the DataFetchingEnvironment.

It looks at the method's last argument type to decide if it should pass the DataFetchingEnvironment as-is or extract the custom context. However in the case of suspending function the actual last argument will be the Continuation, not the DataFetchingEnvironment/Context.

The bug is silent unless you resolver is simultaneously suspending and using a custom context because of the generous "else" case that passes the environment even when the "supposedly" expected type is Continuation.

Thanks for the library & let me know if you would like me to make a small PR to fix this bug or would rather tackle it yourselves.

vojtapol commented 4 years ago

PRs are always welcome.