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

Added error logging and error messaging response when errors occur #267

Merged

Conversation

guilhermeblanco
Copy link
Contributor

Att @vojtapol

@guilhermeblanco
Copy link
Contributor Author

This also closes the graphql-java-kickstart/graphql-spring-boot#455

With this code change, I now receive the proper error feedback on all channels:

CLI:

2020-09-08 18:16:05.812  INFO 1 --- [nio-8080-exec-4] c.e.s.r.g.s.u.UserSessionSubscription    : [UserSessionSubscription] Handling subscription trackedAuctionLaneUpdates()
2020-09-08 18:16:05.846 DEBUG 1 --- [nio-8080-exec-4] o.a.a.c.query.AxonServerQueryBus         : Subscription Query requested with subscription Id [e703b015-d48e-4474-a2e9-08f83104c8b4]
2020-09-08 18:16:06.046 DEBUG 1 --- [nio-8080-exec-4] g.k.e.subscriptions.SessionSubscriber    : Subscribe to execution result: graphql.execution.reactive.DelegatingSubscription@5fe021ce
2020-09-08 18:16:06.365 ERROR 1 --- [ault-executor-1] c.e.s.r.g.s.u.UserSessionSubscription    : The mapper returned a null value.
2020-09-08 18:16:06.382 ERROR 1 --- [ault-executor-1] g.k.e.subscriptions.SessionSubscriber    : Subscription error

java.lang.NullPointerException: The mapper returned a null value.
	at java.base/java.util.Objects.requireNonNull(Objects.java:246) ~[na:na]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	|_ checkpoint ⇢ UserSessionSubscription
Stack trace:
		at java.base/java.util.Objects.requireNonNull(Objects.java:246) ~[na:na]
		at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:100) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:73) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:114) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.FluxFilter$FilterSubscriber.onNext(FluxFilter.java:107) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.FluxMap$MapConditionalSubscriber.onNext(FluxMap.java:213) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:73) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1755) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at reactor.core.publisher.MonoCompletionStage.lambda$subscribe$0(MonoCompletionStage.java:86) ~[reactor-core-3.3.5.RELEASE.jar!/:3.3.5.RELEASE]
		at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:859) ~[na:na]
		at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:837) ~[na:na]
		at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506) ~[na:na]
		at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2073) ~[na:na]

Notice the error seems duplicate, but in reality, this block in my code is now working as intended:

        return subscriptionQueryResult.initialResult()
            .concatWith(subscriptionQueryResult.updates())
            .doOnError((e) -> log.error(e.getMessage()))
            .checkpoint("UserSessionSubscription");

Also, on GraphQL response:

{"type":"error","id":"1","payload":{"errors":[{"message":"The mapper returned a null value.","extensions":null,"path":null}]}}

… any reason. Also fixed bug on JSON processing exception that was not being properly reported
@guilhermeblanco
Copy link
Contributor Author

Last commit now fixes 2 problems:

1- Any Json related problem on decoding the incoming message would mean a crash on ApolloSubscriptionConsumer. The reason was because error was using one variable interpolation slot, but was sending two.

2- Whenever the connection init failed, the connection error was not providing any meaningful information. This now adds an error log displaying the exception.

Copy link
Member

@oliemansm oliemansm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

gradle.properties Outdated Show resolved Hide resolved
@oliemansm oliemansm modified the milestones: 9.2.1, 10.0.0, 10.1.0 Sep 30, 2020
@guilhermeblanco
Copy link
Contributor Author

@oliemansm please let me know if there's anything I can assist here.

@@ -23,7 +23,7 @@ public void accept(String request) {
SubscriptionCommand command = commandProvider.getByType(message.getType());
command.apply(session, message);
} catch (JsonProcessingException e) {
log.error("Cannot read subscription command '{}'", request, e);
log.error("Cannot read subscription command '{}': {}", request, e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add the : {} part here? Since the way it was worked just fine letting it print the stack trace.

@@ -19,6 +19,7 @@ public void apply(SubscriptionSession session, OperationMessage message) {
connectionListeners.forEach(it -> it.onConnect(session, message));
session.sendMessage(new OperationMessage(Type.GQL_CONNECTION_ACK, message.getId(), null));
} catch (Throwable t) {
log.error("Cannot initialize subscription command '{}': {}", message, t);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add the : {} part here? Since the way it was worked just fine letting it print the stack trace.

@oliemansm oliemansm merged commit c5609b7 into graphql-java-kickstart:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants