graphql-java-kickstart/graphql-spring-boot

implementation is not respecting ExceptionHandler specification

nort3x opened this issue · 1 comments

as mentioned in readme file graphql supports exception handling through @ExceptionHandler and i assume it's org.springframework.web.bind.annotation.ExceptionHandler because graphql is not providing any other @ExceptionHandler

now let's take a look at @org.springframework.web.bind.annotation.ExceptionHandler docs

Annotation for handling exceptions in specific handler classes and/or
handler methods.

Handler methods which are annotated with this annotation are allowed to have very flexible signatures. They may have parameters of the following types, in arbitrary order:

  • An exception argument: declared as a general Exception or as a more specific exception. This also serves as a mapping hint if the annotation itself does not narrow the exception types through its {@link #value()}. You may refer to a top-level exception being propagated or to a nested cause within a wrapper exception. As of 5.3, any cause level is being exposed, whereas previously only an immediate cause was considered.
  • Request and/or response objects (typically from the Servlet API). You may choose any specific request/response type, e.g. {@link javax.servlet.ServletRequest} / {@link javax.servlet.http.HttpServletRequest}.
  • Session object: typically {@link javax.servlet.http.HttpSession}. An argument of this type will enforce the presence of a corresponding session. As a consequence, such an argument will never be {@code null}. Note that session access may not be thread-safe, in particular in a Servlet environment: Consider switching the {@link org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter#setSynchronizeOnSession "synchronizeOnSession"} flag to "true" if multiple requests are allowed to access a session concurrently.
  • {@link org.springframework.web.context.request.WebRequest} or {@link org.springframework.web.context.request.NativeWebRequest}. Allows for generic request parameter access as well as request/session attribute access, without ties to the native Servlet API.
  • {@link java.util.Locale} for the current request locale (determined by the most specific locale resolver available, i.e. the configured {@link org.springframework.web.servlet.LocaleResolver} in a Servlet environment).
  • {@link java.io.InputStream} / {@link java.io.Reader} for access to the request's content. This will be the raw InputStream/Reader as exposed by the Servlet API.
  • {@link java.io.OutputStream} / {@link java.io.Writer} for generating the response's content. This will be the raw OutputStream/Writer as exposed by the Servlet API.
  • {@link org.springframework.ui.Model} as an alternative to returning a model map from the handler method. Note that the provided model is not pre-populated with regular model attributes and therefore always empty, as a convenience for preparing the model for an exception-specific view.

The following return types are supported for handler methods:

  • A {@code ModelAndView} object (from Servlet MVC).
  • A {@link org.springframework.ui.Model} object, with the view name implicitly determined through a {@link org.springframework.web.servlet.RequestToViewNameTranslator}.
  • A {@link java.util.Map} object for exposing a model, with the view name implicitly determined through a {@link org.springframework.web.servlet.RequestToViewNameTranslator}.
  • A {@link org.springframework.web.servlet.View} object.
  • A {@link String} value which is interpreted as view name.
  • {@link ResponseBody @responsebody} annotated methods (Servlet-only) to set the response content. The return value will be converted to the response stream using {@linkplain org.springframework.http.converter.HttpMessageConverter message converters}.
  • An {@link org.springframework.http.HttpEntity HttpEntity<?>} or {@link org.springframework.http.ResponseEntity ResponseEntity<?>} object (Servlet-only) to set response headers and content. The ResponseEntity body will be converted and written to the response stream using {@linkplain org.springframework.http.converter.HttpMessageConverter message converters}.
  • {@code void} if the method handles the response itself (by writing the response content directly, declaring an argument of type {@link javax.servlet.ServletResponse} / {@link javax.servlet.http.HttpServletResponse} for that purpose) or if the view name is supposed to be implicitly determined through a {@link org.springframework.web.servlet.RequestToViewNameTranslator} (not declaring a response argument in the handler method signature).

You may combine the {@code ExceptionHandler} annotation with {@link ResponseStatus @ResponseStatus} for a specific HTTP error status.

so it states consumer is allowed to have different method signatures and return types, beside that it can be combined with @ResponseStatus (which i don't know if graphql specifications specifically states that it allows any other status code rather than 200 or not )

this works:

   @ExceptionHandler(AccessDeniedException::class)
    fun accessDeniedExceptionHandler(
        ex: AccessDeniedException
    ): ThrowableGraphQLError {
        // logic
        return ThrowableGraphQLError(ex)
    }

this one results in below output:

   @ExceptionHandler(AccessDeniedException::class)
    fun accessDeniedExceptionHandler(
        ex: AccessDeniedException,
        request: HttpServletRequest
    ): ThrowableGraphQLError {
        // logic
        return ThrowableGraphQLError(ex)
    }
{
  "errors": [
    {
      "message": "argument type mismatch",
      "locations": []
    }
  ],
  "data": null
}

i'm respecting ExceptionHandler specification and in return i expect provider those the same

I'm closing this issue because i didn't get a response in last month ( naturally i assume making a PR have the same fate as well)

i also found spring project official graphql module is much more stable and flexible to it's environment

https://github.com/spring-projects/spring-graphql