From 3ae645bad0b1bba408153b190096392976231fa0 Mon Sep 17 00:00:00 2001 From: Lukas Bloder Date: Fri, 12 Jan 2024 09:09:44 +0100 Subject: [PATCH] Graphql Use new async `handleException` method (#3090) * WIP: initial impl of handling errors in "handleException" instead of "onException" keep "onException" for backwards compat * Format code * add changelog * revert tests for compatibility with graphql-java 17.3 * CR * remove @Nullable annotation and return empty result in SentryDataFetcherExceptionHandler onException --------- Co-authored-by: Sentry Github Bot Co-authored-by: Alexander Dinauer --- CHANGELOG.md | 1 + sentry-graphql/api/sentry-graphql.api | 4 ++- .../SentryDataFetcherExceptionHandler.java | 27 ++++++++++++++++--- ...tryGenericDataFetcherExceptionHandler.java | 22 +++++++++++++-- .../SentryGraphqlExceptionHandler.java | 6 ++--- .../sentry/graphql/SentryInstrumentation.java | 11 +++++--- .../SentryDataFetcherExceptionHandlerTest.kt | 2 +- ...yGenericDataFetcherExceptionHandlerTest.kt | 2 +- .../SentryInstrumentationAnotherTest.kt | 7 ++++- .../graphql/SentryInstrumentationTest.kt | 7 ++++- ...ryDataFetcherExceptionResolverAdapter.java | 7 +++-- ...ryDataFetcherExceptionResolverAdapter.java | 7 +++-- 12 files changed, 82 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64d4181c87..b071654c86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096)) +- Add support for `graphql-java` version 21 ([#3090](https://github.com/getsentry/sentry-java/pull/3090)) ### Fixes diff --git a/sentry-graphql/api/sentry-graphql.api b/sentry-graphql/api/sentry-graphql.api index 58e864d0b4..57c253e23c 100644 --- a/sentry-graphql/api/sentry-graphql.api +++ b/sentry-graphql/api/sentry-graphql.api @@ -32,18 +32,20 @@ public final class io/sentry/graphql/NoOpSubscriptionHandler : io/sentry/graphql public final class io/sentry/graphql/SentryDataFetcherExceptionHandler : graphql/execution/DataFetcherExceptionHandler { public fun (Lgraphql/execution/DataFetcherExceptionHandler;)V public fun (Lio/sentry/IHub;Lgraphql/execution/DataFetcherExceptionHandler;)V + public fun handleException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Ljava/util/concurrent/CompletableFuture; public fun onException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult; } public final class io/sentry/graphql/SentryGenericDataFetcherExceptionHandler : graphql/execution/DataFetcherExceptionHandler { public fun (Lgraphql/execution/DataFetcherExceptionHandler;)V public fun (Lio/sentry/IHub;Lgraphql/execution/DataFetcherExceptionHandler;)V + public fun handleException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Ljava/util/concurrent/CompletableFuture; public fun onException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult; } public final class io/sentry/graphql/SentryGraphqlExceptionHandler { public fun (Lgraphql/execution/DataFetcherExceptionHandler;)V - public fun onException (Ljava/lang/Throwable;Lgraphql/schema/DataFetchingEnvironment;Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult; + public fun handleException (Ljava/lang/Throwable;Lgraphql/schema/DataFetchingEnvironment;Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Ljava/util/concurrent/CompletableFuture; } public final class io/sentry/graphql/SentryInstrumentation : graphql/execution/instrumentation/SimpleInstrumentation { diff --git a/sentry-graphql/src/main/java/io/sentry/graphql/SentryDataFetcherExceptionHandler.java b/sentry-graphql/src/main/java/io/sentry/graphql/SentryDataFetcherExceptionHandler.java index 1e06d0b322..c0467c0089 100644 --- a/sentry-graphql/src/main/java/io/sentry/graphql/SentryDataFetcherExceptionHandler.java +++ b/sentry-graphql/src/main/java/io/sentry/graphql/SentryDataFetcherExceptionHandler.java @@ -10,7 +10,10 @@ import io.sentry.IHub; import io.sentry.SentryIntegrationPackageStorage; import io.sentry.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * Captures exceptions that occur during data fetching, passes them to Sentry and invokes a delegate @@ -36,13 +39,29 @@ public SentryDataFetcherExceptionHandler(final @NotNull DataFetcherExceptionHand } @Override - @SuppressWarnings("deprecation") - public DataFetcherExceptionHandlerResult onException( - final @NotNull DataFetcherExceptionHandlerParameters handlerParameters) { + public CompletableFuture handleException( + DataFetcherExceptionHandlerParameters handlerParameters) { final Hint hint = new Hint(); hint.set(GRAPHQL_HANDLER_PARAMETERS, handlerParameters); hub.captureException(handlerParameters.getException(), hint); - return delegate.onException(handlerParameters); + return delegate.handleException(handlerParameters); + } + + @SuppressWarnings("deprecation") + public DataFetcherExceptionHandlerResult onException( + final @NotNull DataFetcherExceptionHandlerParameters handlerParameters) { + final @Nullable CompletableFuture futureResult = + handleException(handlerParameters); + + if (futureResult != null) { + try { + return futureResult.get(); + } catch (InterruptedException | ExecutionException e) { + return DataFetcherExceptionHandlerResult.newResult().build(); + } + } else { + return DataFetcherExceptionHandlerResult.newResult().build(); + } } } diff --git a/sentry-graphql/src/main/java/io/sentry/graphql/SentryGenericDataFetcherExceptionHandler.java b/sentry-graphql/src/main/java/io/sentry/graphql/SentryGenericDataFetcherExceptionHandler.java index 4b7d72e2cd..6251d00779 100644 --- a/sentry-graphql/src/main/java/io/sentry/graphql/SentryGenericDataFetcherExceptionHandler.java +++ b/sentry-graphql/src/main/java/io/sentry/graphql/SentryGenericDataFetcherExceptionHandler.java @@ -4,6 +4,8 @@ import graphql.execution.DataFetcherExceptionHandlerParameters; import graphql.execution.DataFetcherExceptionHandlerResult; import io.sentry.IHub; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -24,11 +26,27 @@ public SentryGenericDataFetcherExceptionHandler( this(null, delegate); } - @Override @SuppressWarnings("deprecation") public @Nullable DataFetcherExceptionHandlerResult onException( final @NotNull DataFetcherExceptionHandlerParameters handlerParameters) { - return handler.onException( + CompletableFuture futureResult = + handleException(handlerParameters); + + if (futureResult != null) { + try { + return futureResult.get(); + } catch (InterruptedException | ExecutionException e) { + return null; + } + } else { + return null; + } + } + + @Override + public @Nullable CompletableFuture handleException( + DataFetcherExceptionHandlerParameters handlerParameters) { + return handler.handleException( handlerParameters.getException(), handlerParameters.getDataFetchingEnvironment(), handlerParameters); diff --git a/sentry-graphql/src/main/java/io/sentry/graphql/SentryGraphqlExceptionHandler.java b/sentry-graphql/src/main/java/io/sentry/graphql/SentryGraphqlExceptionHandler.java index 527935f863..a1f94cacce 100644 --- a/sentry-graphql/src/main/java/io/sentry/graphql/SentryGraphqlExceptionHandler.java +++ b/sentry-graphql/src/main/java/io/sentry/graphql/SentryGraphqlExceptionHandler.java @@ -8,6 +8,7 @@ import graphql.execution.DataFetcherExceptionHandlerResult; import graphql.schema.DataFetchingEnvironment; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -22,8 +23,7 @@ public SentryGraphqlExceptionHandler(final @Nullable DataFetcherExceptionHandler this.delegate = delegate; } - @SuppressWarnings("deprecation") - public @Nullable DataFetcherExceptionHandlerResult onException( + public @Nullable CompletableFuture handleException( final @NotNull Throwable throwable, final @Nullable DataFetchingEnvironment environment, final @Nullable DataFetcherExceptionHandlerParameters handlerParameters) { @@ -40,7 +40,7 @@ public SentryGraphqlExceptionHandler(final @Nullable DataFetcherExceptionHandler } } if (delegate != null) { - return delegate.onException(handlerParameters); + return delegate.handleException(handlerParameters); } else { return null; } diff --git a/sentry-graphql/src/main/java/io/sentry/graphql/SentryInstrumentation.java b/sentry-graphql/src/main/java/io/sentry/graphql/SentryInstrumentation.java index bd53bb57f2..d2d62c99d8 100644 --- a/sentry-graphql/src/main/java/io/sentry/graphql/SentryInstrumentation.java +++ b/sentry-graphql/src/main/java/io/sentry/graphql/SentryInstrumentation.java @@ -8,7 +8,6 @@ import graphql.execution.ExecutionStepInfo; import graphql.execution.instrumentation.InstrumentationContext; import graphql.execution.instrumentation.InstrumentationState; -import graphql.execution.instrumentation.SimpleInstrumentation; import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters; import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; @@ -37,7 +36,9 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class SentryInstrumentation extends SimpleInstrumentation { +@SuppressWarnings("deprecation") +public final class SentryInstrumentation + extends graphql.execution.instrumentation.SimpleInstrumentation { private static final @NotNull List ERROR_TYPES_HANDLED_BY_DATA_FETCHERS = Arrays.asList( @@ -161,11 +162,13 @@ public SentryInstrumentation( } @Override + @SuppressWarnings("deprecation") public @NotNull InstrumentationState createState() { return new TracingState(); } @Override + @SuppressWarnings("deprecation") public @NotNull InstrumentationContext beginExecution( final @NotNull InstrumentationExecutionParameters parameters) { final TracingState tracingState = parameters.getInstrumentationState(); @@ -176,6 +179,7 @@ public SentryInstrumentation( } @Override + @SuppressWarnings("deprecation") public CompletableFuture instrumentExecutionResult( ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { return super.instrumentExecutionResult(executionResult, parameters) @@ -246,6 +250,7 @@ private boolean isIgnored(final @Nullable String errorType) { } @Override + @SuppressWarnings("deprecation") public @NotNull InstrumentationContext beginExecuteOperation( final @NotNull InstrumentationExecuteOperationParameters parameters) { final @Nullable ExecutionContext executionContext = parameters.getExecutionContext(); @@ -276,7 +281,7 @@ private boolean isIgnored(final @Nullable String errorType) { } @Override - @SuppressWarnings("FutureReturnValueIgnored") + @SuppressWarnings({"FutureReturnValueIgnored", "deprecation"}) public @NotNull DataFetcher instrumentDataFetcher( final @NotNull DataFetcher dataFetcher, final @NotNull InstrumentationFieldFetchParameters parameters) { diff --git a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryDataFetcherExceptionHandlerTest.kt b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryDataFetcherExceptionHandlerTest.kt index 0157c63819..b571fa8218 100644 --- a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryDataFetcherExceptionHandlerTest.kt +++ b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryDataFetcherExceptionHandlerTest.kt @@ -23,6 +23,6 @@ class SentryDataFetcherExceptionHandlerTest { handler.onException(parameters) verify(hub).captureException(eq(exception), anyOrNull()) - verify(delegate).onException(parameters) + verify(delegate).handleException(parameters) } } diff --git a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryGenericDataFetcherExceptionHandlerTest.kt b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryGenericDataFetcherExceptionHandlerTest.kt index 62ac134212..6d643baf01 100644 --- a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryGenericDataFetcherExceptionHandlerTest.kt +++ b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryGenericDataFetcherExceptionHandlerTest.kt @@ -36,6 +36,6 @@ class SentryGenericDataFetcherExceptionHandlerTest { assertNotNull(exceptions) assertEquals(1, exceptions.size) assertEquals(exception, exceptions.first()) - verify(delegate).onException(parameters) + verify(delegate).handleException(parameters) } } diff --git a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationAnotherTest.kt b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationAnotherTest.kt index b8bc76388c..e30bbc9415 100644 --- a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationAnotherTest.kt +++ b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationAnotherTest.kt @@ -131,7 +131,12 @@ class SentryInstrumentationAnotherTest { it.transaction = activeSpan } } - fieldFetchParameters = InstrumentationFieldFetchParameters(executionContext, environment, executionStrategyParameters, false).withNewState( + fieldFetchParameters = InstrumentationFieldFetchParameters( + executionContext, + environment, + executionStrategyParameters, + false + ).withNewState( instrumentationState ) val executionInput = ExecutionInput.newExecutionInput() diff --git a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt index 2294a6aa9c..8a579e2687 100644 --- a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt +++ b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt @@ -193,7 +193,12 @@ class SentryInstrumentationTest { .fields(MergedSelectionSet.newMergedSelectionSet().build()) .field(MergedField.newMergedField().addField(Field.newField("myFieldName").build()).build()) .build() - val parameters = InstrumentationFieldFetchParameters(executionContext, environment, executionStrategyParameters, false).withNewState(SentryInstrumentation.TracingState()) + val parameters = InstrumentationFieldFetchParameters( + executionContext, + environment, + executionStrategyParameters, + false + ).withNewState(SentryInstrumentation.TracingState()) val instrumentedDataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, parameters) val result = instrumentedDataFetcher.get(environment) diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/graphql/SentryDataFetcherExceptionResolverAdapter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/graphql/SentryDataFetcherExceptionResolverAdapter.java index c70830e52a..3f34931147 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/graphql/SentryDataFetcherExceptionResolverAdapter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/graphql/SentryDataFetcherExceptionResolverAdapter.java @@ -5,6 +5,7 @@ import graphql.schema.DataFetchingEnvironment; import io.sentry.graphql.SentryGraphqlExceptionHandler; import java.util.List; +import java.util.concurrent.CompletableFuture; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -36,9 +37,11 @@ public boolean isThreadLocalContextAware() { @Override protected @Nullable List resolveToMultipleErrors( Throwable ex, DataFetchingEnvironment env) { - @Nullable DataFetcherExceptionHandlerResult result = handler.onException(ex, env, null); + @Nullable + CompletableFuture result = + handler.handleException(ex, env, null); if (result != null) { - return result.getErrors(); + return result.join().getErrors(); } return null; } diff --git a/sentry-spring/src/main/java/io/sentry/spring/graphql/SentryDataFetcherExceptionResolverAdapter.java b/sentry-spring/src/main/java/io/sentry/spring/graphql/SentryDataFetcherExceptionResolverAdapter.java index c52823653e..16ebe0ce8c 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/graphql/SentryDataFetcherExceptionResolverAdapter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/graphql/SentryDataFetcherExceptionResolverAdapter.java @@ -5,6 +5,7 @@ import graphql.schema.DataFetchingEnvironment; import io.sentry.graphql.SentryGraphqlExceptionHandler; import java.util.List; +import java.util.concurrent.CompletableFuture; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -36,9 +37,11 @@ public boolean isThreadLocalContextAware() { @Override protected @Nullable List resolveToMultipleErrors( Throwable ex, DataFetchingEnvironment env) { - @Nullable DataFetcherExceptionHandlerResult result = handler.onException(ex, env, null); + @Nullable + CompletableFuture result = + handler.handleException(ex, env, null); if (result != null) { - return result.getErrors(); + return result.join().getErrors(); } return null; }