Fix: Sending errors in Spring WebFlux integration

getsentry · Jan 5, 2022 · 5f52d18 · 5f52d18
1 parent 9f87477
commit 5f52d18
Show file tree

Hide file tree

Showing 11 changed files with 165 additions and 105 deletions.
diff --git a/...les-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/PersonController.java b/...les-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/PersonController.java
@@ -1,5 +1,7 @@
 package io.sentry.samples.spring.boot;
 
+import static io.sentry.spring.webflux.SentryReactor.withSentry;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.bind.annotation.GetMapping;
@@ -21,9 +23,13 @@ public PersonController(PersonService personService) {
   }
 
   @GetMapping("{id}")
-  Person person(@PathVariable Long id) {
-    LOGGER.info("Loading person with id={}", id);
-    throw new IllegalArgumentException("Something went wrong [id=" + id + "]");
+  Mono<Long> person(@PathVariable Long id) {
+    return Mono.just(id)
+        .doOnEach(withSentry(it -> LOGGER.info("Loading person with id={}", id)))
+        .doOnNext(
+            it -> {
+              throw new IllegalArgumentException("Something went wrong [id=" + id + "]");
+            });
   }
 
   @PostMapping

diff --git a/...amples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/PersonService.java b/...amples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/PersonService.java
@@ -1,5 +1,7 @@
 package io.sentry.samples.spring.boot;
 
+import static io.sentry.spring.webflux.SentryReactor.withSentry;
+
 import io.sentry.Sentry;
 import java.time.Duration;
 import org.springframework.stereotype.Service;
@@ -12,7 +14,7 @@ public class PersonService {
   Mono<Person> create(Person person) {
     return Mono.delay(Duration.ofMillis(100))
         .publishOn(Schedulers.boundedElastic())
-        .doOnNext(__ -> Sentry.captureMessage("Creating person"))
+        .doOnEach(withSentry(__ -> Sentry.captureMessage("Creating person")))
         .map(__ -> person);
   }
 }
diff --git a/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api b/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api
@@ -44,8 +44,7 @@ public class io/sentry/spring/boot/SentryProperties$Logging {
 
 public class io/sentry/spring/boot/SentryWebfluxAutoConfiguration {
 	public fun <init> ()V
-	public fun sentryScheduleHookApplicationRunner ()Lorg/springframework/boot/ApplicationRunner;
-	public fun sentryWebExceptionHandler (Lio/sentry/IHub;)Lio/sentry/spring/webflux/SentryWebExceptionHandler;
-	public fun sentryWebFilter (Lio/sentry/IHub;)Lio/sentry/spring/webflux/SentryWebFilter;
+	public fun sentryWebExceptionHandler ()Lio/sentry/spring/webflux/SentryWebExceptionHandler;
+	public fun sentryWebFilter ()Lio/sentry/spring/webflux/SentryWebFilter;
 }
 
diff --git a/...ring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java b/...ring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java
@@ -2,14 +2,13 @@
 
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.IHub;
-import io.sentry.spring.webflux.SentryScheduleHook;
 import io.sentry.spring.webflux.SentryWebExceptionHandler;
 import io.sentry.spring.webflux.SentryWebFilter;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
-import org.springframework.boot.ApplicationRunner;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -19,28 +18,21 @@
 @Configuration(proxyBeanMethods = false)
 @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.REACTIVE)
 @ConditionalOnBean(IHub.class)
+@ConditionalOnProperty(name = "sentry.dsn")
 @ConditionalOnClass(Schedulers.class)
 @Open
 @ApiStatus.Experimental
 public class SentryWebfluxAutoConfiguration {
 
-  /** Configures hook that sets correct hub on the executing thread. */
-  @Bean
-  public @NotNull ApplicationRunner sentryScheduleHookApplicationRunner() {
-    return args -> {
-      Schedulers.onScheduleHook("sentry", new SentryScheduleHook());
-    };
-  }
-
   /** Configures a filter that sets up Sentry {@link io.sentry.Scope} for each request. */
   @Bean
-  public @NotNull SentryWebFilter sentryWebFilter(final @NotNull IHub hub) {
-    return new SentryWebFilter(hub);
+  public @NotNull SentryWebFilter sentryWebFilter() {
+    return new SentryWebFilter();
   }
 
   /** Configures exception handler that handles unhandled exceptions and sends them to Sentry. */
   @Bean
-  public @NotNull SentryWebExceptionHandler sentryWebExceptionHandler(final @NotNull IHub hub) {
-    return new SentryWebExceptionHandler(hub);
+  public @NotNull SentryWebExceptionHandler sentryWebExceptionHandler() {
+    return new SentryWebExceptionHandler();
   }
 }
diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api
@@ -161,24 +161,26 @@ public final class io/sentry/spring/tracing/TransactionNameProvider {
 	public fun provideTransactionName (Ljavax/servlet/http/HttpServletRequest;)Ljava/lang/String;
 }
 
-public class io/sentry/spring/webflux/SentryRequestResolver {
-	public fun <init> (Lio/sentry/IHub;)V
-	public fun resolveSentryRequest (Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/protocol/Request;
+public final class io/sentry/spring/webflux/SentryReactor {
+	public fun <init> ()V
+	public static fun withSentry (Ljava/util/function/Consumer;)Ljava/util/function/Consumer;
 }
 
-public final class io/sentry/spring/webflux/SentryScheduleHook : java/util/function/Function {
+public class io/sentry/spring/webflux/SentryRequestResolver {
 	public fun <init> ()V
-	public synthetic fun apply (Ljava/lang/Object;)Ljava/lang/Object;
-	public fun apply (Ljava/lang/Runnable;)Ljava/lang/Runnable;
+	public fun resolveSentryRequest (Lio/sentry/SentryOptions;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/protocol/Request;
 }
 
 public final class io/sentry/spring/webflux/SentryWebExceptionHandler : org/springframework/web/server/WebExceptionHandler {
-	public fun <init> (Lio/sentry/IHub;)V
+	public fun <init> ()V
 	public fun handle (Lorg/springframework/web/server/ServerWebExchange;Ljava/lang/Throwable;)Lreactor/core/publisher/Mono;
 }
 
 public final class io/sentry/spring/webflux/SentryWebFilter : org/springframework/web/server/WebFilter {
-	public fun <init> (Lio/sentry/IHub;)V
+	public static field HUB_EXCHANGE_CONTEXT_ATTRIBUTE Ljava/lang/String;
+	public static field HUB_REACTOR_CONTEXT_ATTRIBUTE Ljava/lang/String;
+	public fun <init> ()V
 	public fun filter (Lorg/springframework/web/server/ServerWebExchange;Lorg/springframework/web/server/WebFilterChain;)Lreactor/core/publisher/Mono;
+	public static fun getHub (Lreactor/util/context/ContextView;)Ljava/util/Optional;
 }
 
diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryReactor.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryReactor.java
@@ -0,0 +1,39 @@
+package io.sentry.spring.webflux;
+
+import io.sentry.IHub;
+import io.sentry.Sentry;
+import java.util.function.Consumer;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+import reactor.core.publisher.Signal;
+
+@ApiStatus.Experimental
+public final class SentryReactor {
+
+  /**
+   * Takes the Sentry {@link IHub} associated with the HTTP request and sets it on current thread
+   * for the time of executing {@link Consumer} given by parameter.
+   *
+   * @param consumer - the consumer to execute
+   * @param <T> type of signal
+   * @return a consumer of signal
+   */
+  public static <T> @NotNull Consumer<Signal<T>> withSentry(final @NotNull Consumer<T> consumer) {
+    return signal -> {
+      if (!signal.isOnNext()) return;
+      final IHub currentHub = Sentry.getCurrentHub();
+      final IHub contextViewHub =
+          signal.getContextView().getOrDefault(SentryWebFilter.HUB_REACTOR_CONTEXT_ATTRIBUTE, null);
+      if (contextViewHub != null) {
+        Sentry.setCurrentHub(contextViewHub);
+        try {
+          consumer.accept(signal.get());
+        } finally {
+          Sentry.setCurrentHub(currentHub);
+        }
+      } else {
+        consumer.accept(signal.get());
+      }
+    };
+  }
+}
diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java
@@ -1,9 +1,8 @@
 package io.sentry.spring.webflux;
 
 import com.jakewharton.nopen.annotation.Open;
-import io.sentry.IHub;
+import io.sentry.SentryOptions;
 import io.sentry.protocol.Request;
-import io.sentry.util.Objects;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -21,31 +20,27 @@ public class SentryRequestResolver {
   private static final List<String> SENSITIVE_HEADERS =
       Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE");
 
-  private final @NotNull IHub hub;
-
-  public SentryRequestResolver(final @NotNull IHub hub) {
-    this.hub = Objects.requireNonNull(hub, "options is required");
-  }
-
-  public @NotNull Request resolveSentryRequest(final @NotNull ServerHttpRequest httpRequest) {
+  public @NotNull Request resolveSentryRequest(
+      final @NotNull SentryOptions options, final @NotNull ServerHttpRequest httpRequest) {
     final Request sentryRequest = new Request();
     sentryRequest.setMethod(httpRequest.getMethodValue());
     sentryRequest.setQueryString(httpRequest.getURI().getQuery());
     sentryRequest.setUrl(httpRequest.getURI().toString());
-    sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders()));
+    sentryRequest.setHeaders(resolveHeadersMap(options, httpRequest.getHeaders()));
 
-    if (hub.getOptions().isSendDefaultPii()) {
+    if (options.isSendDefaultPii()) {
       sentryRequest.setCookies(toString(httpRequest.getHeaders().get("Cookies")));
     }
     return sentryRequest;
   }
 
   @NotNull
-  Map<String, String> resolveHeadersMap(final HttpHeaders request) {
+  Map<String, String> resolveHeadersMap(
+      final @NotNull SentryOptions options, final HttpHeaders request) {
     final Map<String, String> headersMap = new HashMap<>();
     for (Map.Entry<String, List<String>> entry : request.entrySet()) {
       // do not copy personal information identifiable headers
-      if (hub.getOptions().isSendDefaultPii()
+      if (options.isSendDefaultPii()
           || !SENSITIVE_HEADERS.contains(entry.getKey().toUpperCase(Locale.ROOT))) {
         headersMap.put(entry.getKey(), toString(entry.getValue()));
       }

diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryScheduleHook.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryScheduleHook.java
diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebExceptionHandler.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebExceptionHandler.java
@@ -5,7 +5,6 @@
 import io.sentry.SentryLevel;
 import io.sentry.exception.ExceptionMechanismException;
 import io.sentry.protocol.Mechanism;
-import io.sentry.util.Objects;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.core.annotation.Order;
@@ -20,11 +19,6 @@
 // at -1
 @ApiStatus.Experimental
 public final class SentryWebExceptionHandler implements WebExceptionHandler {
-  private final @NotNull IHub hub;
-
-  public SentryWebExceptionHandler(final @NotNull IHub hub) {
-    this.hub = Objects.requireNonNull(hub, "hub is required");
-  }
 
   @Override
   public @NotNull Mono<Void> handle(
@@ -38,7 +32,12 @@ public SentryWebExceptionHandler(final @NotNull IHub hub) {
       final SentryEvent event = new SentryEvent(throwable);
       event.setLevel(SentryLevel.FATAL);
       event.setTransaction(TransactionNameProvider.provideTransactionName(serverWebExchange));
-      hub.captureEvent(event);
+      final IHub hub =
+          (IHub)
+              serverWebExchange.getAttributes().get(SentryWebFilter.HUB_EXCHANGE_CONTEXT_ATTRIBUTE);
+      if (hub != null) {
+        hub.captureEvent(event);
+      }
     }
     return Mono.error(ex);
   }

diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java
@@ -2,44 +2,86 @@
 
 import io.sentry.Breadcrumb;
 import io.sentry.IHub;
-import io.sentry.util.Objects;
+import io.sentry.Sentry;
+import java.util.Optional;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebFilter;
 import org.springframework.web.server.WebFilterChain;
 import reactor.core.publisher.Mono;
+import reactor.util.context.Context;
+import reactor.util.context.ContextView;
 
 /** Manages {@link io.sentry.Scope} in Webflux request processing. */
 @ApiStatus.Experimental
 public final class SentryWebFilter implements WebFilter {
-  private final @NotNull IHub hub;
+  /**
+   * A key under which current {@link io.sentry.IHub} is stored in Spring Webflux {@link
+   * org.springframework.web.server.ServerWebExchange}.
+   */
+  public static String HUB_EXCHANGE_CONTEXT_ATTRIBUTE =
+      SentryWebFilter.class.getName() + ".EXCHANGE_CONTEXT";
+
+  /**
+   * A key under which current {@link io.sentry.IHub} is stored in Reactor Context {@link Context}.
+   */
+  public static String HUB_REACTOR_CONTEXT_ATTRIBUTE =
+      SentryWebFilter.class.getName() + ".REACTOR_CONTEXT";
+
   private final @NotNull SentryRequestResolver sentryRequestResolver;
 
-  public SentryWebFilter(final @NotNull IHub hub) {
-    this.hub = Objects.requireNonNull(hub, "hub is required");
-    this.sentryRequestResolver = new SentryRequestResolver(hub);
+  public SentryWebFilter() {
+    this.sentryRequestResolver = new SentryRequestResolver();
   }
 
   @Override
   public Mono<Void> filter(
       final @NotNull ServerWebExchange serverWebExchange,
       final @NotNull WebFilterChain webFilterChain) {
+    // hub used in request execution, can be retrieved from ServerWebExchange or Reactor Context
+    final IHub currentHub = Sentry.getCurrentHub().clone();
+    serverWebExchange.getAttributes().put(HUB_EXCHANGE_CONTEXT_ATTRIBUTE, currentHub);
     return webFilterChain
         .filter(serverWebExchange)
         .doFinally(
             __ -> {
-              hub.popScope();
+              final IHub hub = getHub(serverWebExchange);
+              if (hub != null) {
+                hub.popScope();
+              }
             })
         .doFirst(
             () -> {
-              hub.pushScope();
-              final ServerHttpRequest request = serverWebExchange.getRequest();
-              hub.addBreadcrumb(
-                  Breadcrumb.http(request.getURI().toString(), request.getMethodValue()));
-              hub.configureScope(
-                  scope -> scope.setRequest(sentryRequestResolver.resolveSentryRequest(request)));
-            });
+              final IHub hub = getHub(serverWebExchange);
+              if (hub != null) {
+                hub.pushScope();
+                final ServerHttpRequest request = serverWebExchange.getRequest();
+                hub.addBreadcrumb(
+                    Breadcrumb.http(request.getURI().toString(), request.getMethodValue()));
+                hub.configureScope(
+                    scope ->
+                        scope.setRequest(
+                            sentryRequestResolver.resolveSentryRequest(hub.getOptions(), request)));
+              }
+            })
+        .contextWrite(ctx -> ctx.put(HUB_REACTOR_CONTEXT_ATTRIBUTE, currentHub))
+        .then();
+  }
+
+  private @Nullable IHub getHub(@NotNull ServerWebExchange serverWebExchange) {
+    return (IHub) serverWebExchange.getAttributes().get(HUB_EXCHANGE_CONTEXT_ATTRIBUTE);
+  }
+
+  /**
+   * Resolves Sentry {@link IHub} for current request execution from Reactor {@link Context}.
+   *
+   * @param context - reactor context
+   * @return hub or empty if hub is not assigned to context
+   */
+  public static @NotNull Optional<IHub> getHub(ContextView context) {
+    return context.getOrEmpty(HUB_REACTOR_CONTEXT_ATTRIBUTE);
   }
 }