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

Graphql Use new async handleException method #3090

Merged
merged 11 commits into from
Jan 12, 2024
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Add support for `graphql-java` version 21 ([#3090](https://github.com/getsentry/sentry-java/pull/3090))

### Fixes

- SchedulerFactoryBeanCustomizer now runs first so user customization is not overridden ([#3095](https://github.com/getsentry/sentry-java/pull/3095))
Expand Down
4 changes: 3 additions & 1 deletion sentry-graphql/api/sentry-graphql.api
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -24,11 +26,27 @@ public SentryGenericDataFetcherExceptionHandler(
this(null, delegate);
}

@Override
@SuppressWarnings("deprecation")
public @Nullable DataFetcherExceptionHandlerResult onException(
adinauer marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -40,7 +40,7 @@ public SentryGraphqlExceptionHandler(final @Nullable DataFetcherExceptionHandler
}
}
if (delegate != null) {
return delegate.onException(handlerParameters);
return delegate.handleException(handlerParameters);
} else {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand All @@ -176,6 +179,7 @@ public SentryInstrumentation(
}

@Override
@SuppressWarnings("deprecation")
public CompletableFuture<ExecutionResult> instrumentExecutionResult(
ExecutionResult executionResult, InstrumentationExecutionParameters parameters) {
return super.instrumentExecutionResult(executionResult, parameters)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ class SentryDataFetcherExceptionHandlerTest {
handler.onException(parameters)

verify(hub).captureException(eq(exception), anyOrNull<Hint>())
verify(delegate).onException(parameters)
verify(delegate).handleException(parameters)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ class SentryGenericDataFetcherExceptionHandlerTest {
assertNotNull(exceptions)
assertEquals(1, exceptions.size)
assertEquals(exception, exceptions.first())
verify(delegate).onException(parameters)
verify(delegate).handleException(parameters)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down