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

The detail message from Checked Exceptions thrown by Resolvers is ignored in ExecutionResult.getErrors() #713

Closed
zleonov opened this issue Oct 11, 2022 · 3 comments
Labels

Comments

@zleonov
Copy link

zleonov commented Oct 11, 2022

Description

graphql-java-tools maps fields on GraphQL objects to methods and properties on "Resolvers" and "Data Classes" thus eliminating the boilerplate code required to setup graphql-java manually. Resolver methods are invoked via reflection and are free to throw both checked and unchecked exceptions in their method signatures.

When an exception is thrown it eventually ends up as a GraphQLError object in the ExecutionResult.getErrors() list. For runtime exceptions the resulting GraphQLError object inherits their error messages. But for checked exceptions or Throwables (which the Resolver methods are technically free to throw as well) the detail message is lost and a null is returned.

This problem does not exist in graphql-java.

Expected behavior

The detail message from Checked Exceptions thrown by Resolvers should be propogated to GraphQLError.getMessage() returned from ExecutionResult.getErrors().

Actual behavior

GraphQLError objects corresponding to Checked Exceptions thrown by Resolvers return a null in their getMessage().

Steps to reproduce the bug

  1. Create a simple hello-world demo using a GraphQLQueryResolver (and write the schema accordingly)
  2. Once the demo works, change the hello() method to throw a checked exception with a message.
  3. Examine the errors in ExecutionResult.

I have a gist that clearly demonstrates the bug: https://gist.github.com/zleonov/32f45aefdce49d4666877f4d276d5c2a

@zleonov zleonov added the bug label Oct 11, 2022
@zleonov
Copy link
Author

zleonov commented Oct 11, 2022

I took a cursory look at the source code and I think this is where the bug occurs:

src\main\kotlin\graphql\kickstart\tools\resolver\MethodFieldResolver.kt(267):

@Suppress("NOTHING_TO_INLINE")
private inline fun invoke(method: Method, instance: Any, args: Array<Any?>): Any? {
    try {
        return method.invoke(instance, *args)
    } catch (invocationException: java.lang.reflect.InvocationTargetException) {
        val e = invocationException.cause
        if (e is RuntimeException) {
            throw e
        }
        if (e is Error) {
            throw e
        }

        throw java.lang.reflect.UndeclaredThrowableException(e)
    }
}

UndeclaredThrowableException does not inherit the detailed message from the underlying exception and as far as I can tell that exception is not unwrapped downstream. UndeclaredThrowableException.getMessage() returns a null.

@zleonov
Copy link
Author

zleonov commented Oct 11, 2022

I just realized this may be the same issue as #477

@zleonov zleonov changed the title The getMessage() from Checked Exceptions thrown by Resolvers is ignored in ExecutionResult.getErrors() The detail message from Checked Exceptions thrown by Resolvers is ignored in ExecutionResult.getErrors() Oct 11, 2022
@oryan-block
Copy link
Collaborator

Fixed in #757

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants