`SubscriptionExceptionResolver.resolveException` should accept `DataFetchingEnvironment`

[yassenb]

Currently the method in DataFetcherExceptionResolver is Mono<List<GraphQLError>> resolveException(Throwable exception, DataFetchingEnvironment environment) while the method in SubscriptionExceptionResolver is Mono<List<GraphQLError>> resolveException(Throwable exception). It would be nice to have DataFetchingEnvironment passed in as well to include more information in our error logs.

As a side note in DataFetcherExceptionResolver.resolveException I've noticed that for some reason SecurityContextHolder.getContext().getAuthentication() returns an instance of AnonymousAuthentication when I know the user is authenticated (and the same piece of code works correctly elsewhere) and as a work-around I'm force to use environment.getGraphQlContext().get<SecurityContextImpl>(SecurityContext::class.name).getAuthentication() to get the proper authentication instance. Same applies in SubscriptionExceptionResolver

[rstoyanchev]

This would have to be the original DataFetchingEnvironment at the start of the subscription. I suppose that's the one you have in mind?

for some reason SecurityContextHolder.getContext().getAuthentication() returns an instance of AnonymousAuthentication

If you could open a separate issue. A small reporducer if available would help speed it up.

[yassenb]

This would have to be the original DataFetchingEnvironment at the start of the subscription. I suppose that's the one you have in mind?

I'm currently using getGraphQlContext(), getField() and getArguments(). Not completely sure what you mean by the start but an exception could occur at any point while emitting a result and resolving some field so I would imagine the latter two are "per-emission", not "per-subscription"

If you could open a separate issue

Sure, I will work on a minimal repo

[rstoyanchev]

I have to correct myself a little. I was referring to the fact the @SubscriptionMapping method is called once at the start of request execution. Then it's up to the Publisher to keep producing results. However, it is true that GraphQL Java gets involved in calling data fetchers for related entities for the graph of each result item.

SubscriptionExceptionResolver, however handles error signals from the Publisher, and only has access to the exception that propagated and the DataFetcherEnvironment for the @SubscriptionMapping method. For anything more, exception handling needs to be closer to the invocation of DataFetchers.

GraphQL Java is in the best position there, but it doesn't support anything at that level as far as I know (for subscriptions that is). At our level, we do handle exceptions from every controller method invocation and call @GraphQlExceptionHandler controller methods. Have you considered using that?

[yassenb]

Thank you, with your information and some more digging I was able to achieve what I needed by using @GraphQlExceptionHandler.

Feel free to close this issue if you deem so. Not sure if in the case of subscriptions there's a reason to use SubscriptionExceptionResolver over @ControllerAdvice with @GraphQlExceptionHandler

[rstoyanchev]

SubscriptionExceptionResolver came first, and @GraphQlExceptionHandler after that. At present, I would use the latter for controller methods. The former is at a lower level and can handle exceptions from any other DataFetcher.