Skip to content

FieldAccessException should include field as well as global errors in its message #348

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
busches opened this issue Apr 6, 2022 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@busches
Copy link

busches commented Apr 6, 2022

When making a GraphQl Client call to a field that returns a list of an entity, when then list is empty, we're getting the following error:

 org.springframework.graphql.client.FieldAccessException: Invalid field 'getRefillNumbers', errors: []
	at org.springframework.graphql.client.DefaultGraphQlClient$RetrieveSpecSupport.getValidField(DefaultGraphQlClient.java:183)
	at org.springframework.graphql.client.DefaultGraphQlClient$DefaultRetrieveSpec.lambda$toEntityList$2(DefaultGraphQlClient.java:213)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:113)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1816)
	at reactor.core.publisher.MonoFlatMap$FlatMapInner.onNext(MonoFlatMap.java:249)
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:127)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:127)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1816)
	at reactor.core.publisher.MonoFlatMap$FlatMapInner.onNext(MonoFlatMap.java:249)
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
	at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1816)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:151)
	at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableConditionalSubscriber.onNext(FluxMapFuseable.java:295)
	at reactor.core.publisher.FluxFilterFuseable$FilterFuseableConditionalSubscriber.onNext(FluxFilterFuseable.java:337)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1816)
	at reactor.core.publisher.MonoCollect$CollectSubscriber.onComplete(MonoCollect.java:159)
	at reactor.core.publisher.FluxMap$MapSubscriber.onComplete(FluxMap.java:142)
	at reactor.core.publisher.FluxPeek$PeekSubscriber.onComplete(FluxPeek.java:260)
	at reactor.core.publisher.FluxMap$MapSubscriber.onComplete(FluxMap.java:142)
	at reactor.netty.channel.FluxReceive.onInboundComplete(FluxReceive.java:400)
	at reactor.netty.channel.ChannelOperations.onInboundComplete(ChannelOperations.java:419)
	at reactor.netty.channel.ChannelOperations.terminate(ChannelOperations.java:473)
	at reactor.netty.http.client.HttpClientOperations.onInboundNext(HttpClientOperations.java:703)
	at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:93)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:327)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:299)
	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1372)
	at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1235)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1284)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:510)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:449)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:279)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:800)
	at io.netty.channel.epoll.AbstractEpollChannel$AbstractEpollUnsafe$1.run(AbstractEpollChannel.java:425)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469)
	at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:384)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Unknown Source)

Version: implementation("org.springframework.graphql:spring-graphql:1.0.0-M6")

Calling Code:

 val response = graphQlClient.documentName(documentName)
            .variables(variables)
            .retrieve("getRefillNumbers")
            .toEntityList(GetRefillNumbers::class.java)

Failing response:

{
	"data": {
		"getRefillNumbers": []
	}
}

Successful response:

{
	"data": {
		"getRefillNumbers": [
			{
				"somedata": "1860"
			}
       ]
}

I would not expect an empty array to result in an error, when it still technically follows the schema, additionally, the error thrown is very misleading, since the field getRefillNumbers is still there.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 6, 2022
@rstoyanchev rstoyanchev self-assigned this Apr 11, 2022
@rstoyanchev rstoyanchev added this to the 1.0.0-RC1 milestone Apr 11, 2022
@rstoyanchev
Copy link
Contributor

On closer look, I am unable to reproduce and I can't see why it's happening.

If you look at the line where the exception is raised, the response must be invalid of there is a field error. The former is defined as "data" is not present or null, which is not the case. For the latter, your snippet shows no "errors" at all, and the exception message confirms no field errors with errors [].

Could you please, either debug further based on this information to understand why it's raising the error, or provide a sample? Also, try using the latest snapshot, just in case.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Apr 14, 2022
@rstoyanchev rstoyanchev removed this from the 1.0.0-RC1 milestone Apr 14, 2022
@busches
Copy link
Author

busches commented Apr 15, 2022

@rstoyanchev thanks, I'll double check if errors is populated or not, I'd find it pretty odd if it's there when there's values in the array and not when the array is empty. I likely just left it out of the response, since it also had all the federation stuff with it, but that's an easy thing to test.

Also in your tests, were you using M6 or what version to recreate?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 15, 2022
@busches
Copy link
Author

busches commented Apr 21, 2022

Confirmed that it's only data, no errors block, but I was not able to recreate it in a reproduction project. The production code is still having this issue, so I'll see if using that project I can recreate it.

@busches
Copy link
Author

busches commented Apr 21, 2022

Ok, and I was finally able to recreate it, but the response was only errors with a null data. The error makes sense now, but I had to add more code to trap it to see the actual response bubbled up.

{
	"errors": [
		{
			"message": "401: Unauthorized",
			"extensions": {
				"code": "UNAUTHENTICATED"
			},
			"code": "UNAUTHENTICATED"
		}
	],
	"data": null,
	"extensions": {
		"tracing": {
			"version": 1,
			"startTime": "2022-04-21T14:46:53.732Z",
			"endTime": "2022-04-21T14:46:53.752Z",
			"duration": 20235956,
			"execution": {
				"resolvers": []
			}
		}
	}
}

Confirmed we get the same error in RC1, so the error message looks correct for what I'm seeing, I just wish it'd bubble up the response by default. :)

We're trapping and rethrowing like so:

            .onErrorMap { error ->
                if (error is GraphQlTransportException && error.cause is WebClientResponseException) {
                    val responseBody = (error.cause as WebClientResponseException).responseBodyAsString
                    GraphQLException("GraphQL call failed, ${error.message}, responseBody: $responseBody")
                } else if (error is FieldAccessException) { // This is the new case for the above response
                    GraphQLException("GraphQL call failed, ${error.message}, responseBody: ${error.response}")
                } else {
                    logger.error("Unhandled GraphQl Error, type was ${error.javaClass}")
                    error
                }
            }

Given this, I think we can close this, thanks!

@busches busches closed this as completed Apr 21, 2022
@rstoyanchev
Copy link
Contributor

On closer look, it's true that FieldAccessException can be raised because of a global, request-level error but its message does not take that into account and prints only field errors. I'm re-opening to improve the error message.

@rstoyanchev rstoyanchev reopened this Apr 25, 2022
@rstoyanchev rstoyanchev changed the title GraphQl Client - Invalid Field when empty Array The FieldAccessException message should check both field and global errors Apr 25, 2022
@rstoyanchev rstoyanchev added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Apr 25, 2022
@rstoyanchev rstoyanchev added this to the 1.0.0 milestone Apr 25, 2022
@rstoyanchev rstoyanchev changed the title The FieldAccessException message should check both field and global errors FieldAccessException should include field as well as global errors in its message Apr 25, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants