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 <[email protected]> Co-authored-by: Alexander Dinauer <[email protected]>
getsentry · Jan 12, 2024 · 3ae645b · 3ae645b
1 parent 606074f
commit 3ae645b
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 21 deletions.
diff --git 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
@@ -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 <init> (Lgraphql/execution/DataFetcherExceptionHandler;)V
 	public fun <init> (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 <init> (Lgraphql/execution/DataFetcherExceptionHandler;)V
 	public fun <init> (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 <init> (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
@@ -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<DataFetcherExceptionHandlerResult> 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<DataFetcherExceptionHandlerResult> 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
@@ -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<DataFetcherExceptionHandlerResult> futureResult =
+        handleException(handlerParameters);
+
+    if (futureResult != null) {
+      try {
+        return futureResult.get();
+      } catch (InterruptedException | ExecutionException e) {
+        return null;
+      }
+    } else {
+      return null;
+    }
+  }
+
+  @Override
+  public @Nullable CompletableFuture<DataFetcherExceptionHandlerResult> 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
@@ -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<DataFetcherExceptionHandlerResult> 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
@@ -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<String> 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<ExecutionResult> beginExecution(
       final @NotNull InstrumentationExecutionParameters parameters) {
     final TracingState tracingState = parameters.getInstrumentationState();
@@ -176,6 +179,7 @@ public SentryInstrumentation(
   }
 
   @Override
+  @SuppressWarnings("deprecation")
   public CompletableFuture<ExecutionResult> 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<ExecutionResult> 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
@@ -23,6 +23,6 @@ class SentryDataFetcherExceptionHandlerTest {
         handler.onException(parameters)
 
         verify(hub).captureException(eq(exception), anyOrNull<Hint>())
-        verify(delegate).onException(parameters)
+        verify(delegate).handleException(parameters)
     }
 }
diff --git a/...graphql/src/test/kotlin/io/sentry/graphql/SentryGenericDataFetcherExceptionHandlerTest.kt b/...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
@@ -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
@@ -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/...main/java/io/sentry/spring/jakarta/graphql/SentryDataFetcherExceptionResolverAdapter.java b/...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<GraphQLError> resolveToMultipleErrors(
       Throwable ex, DataFetchingEnvironment env) {
-    @Nullable DataFetcherExceptionHandlerResult result = handler.onException(ex, env, null);
+    @Nullable
+    CompletableFuture<DataFetcherExceptionHandlerResult> result =
+        handler.handleException(ex, env, null);
     if (result != null) {
-      return result.getErrors();
+      return result.join().getErrors();
     }
     return null;
   }

diff --git a/...ing/src/main/java/io/sentry/spring/graphql/SentryDataFetcherExceptionResolverAdapter.java b/...ing/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<GraphQLError> resolveToMultipleErrors(
       Throwable ex, DataFetchingEnvironment env) {
-    @Nullable DataFetcherExceptionHandlerResult result = handler.onException(ex, env, null);
+    @Nullable
+    CompletableFuture<DataFetcherExceptionHandlerResult> result =
+        handler.handleException(ex, env, null);
     if (result != null) {
-      return result.getErrors();
+      return result.join().getErrors();
     }
     return null;
   }