diff --git a/CHANGELOG.md b/CHANGELOG.md index 7228c33cd1..fa4e382d11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Fix: use connection and read timeouts in ApacheHttpClient based transport (#1397) * Ref: Refactor converting HttpServletRequest to Sentry Request in Spring integration (#1387) +* Fix: handle network errors in SentrySpanClientHttpRequestInterceptor (#1407) # 4.4.0-alpha.2 diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt index 08b1376786..4abb84168a 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt @@ -8,6 +8,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import java.io.IOException import kotlin.test.Test import org.assertj.core.api.Assertions.assertThat import org.springframework.http.HttpMethod @@ -31,7 +32,7 @@ class SentrySpanRestTemplateCustomizerTest { whenever(hub.options).thenReturn(sentryOptions) } - fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK): RestTemplate { + fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK, throwIOException: Boolean = false): RestTemplate { customizer.customize(restTemplate) if (isTransactionActive) { @@ -39,7 +40,7 @@ class SentrySpanRestTemplateCustomizerTest { scope.setTransaction(transaction) whenever(hub.span).thenReturn(transaction) - mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) + val scenario = mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) .andExpect(MockRestRequestMatchers.method(HttpMethod.GET)) .andExpect { // must have trace id from the parent transaction and must not contain spanId from the parent transaction @@ -47,7 +48,13 @@ class SentrySpanRestTemplateCustomizerTest { .endsWith("-1") .doesNotContain(transaction.spanContext.spanId.toString()) } - .andRespond(MockRestResponseCreators.withStatus(status).body("OK").contentType(MediaType.APPLICATION_JSON)) + if (throwIOException) { + scenario.andRespond { + throw IOException() + } + } else { + scenario.andRespond(MockRestResponseCreators.withStatus(status).body("OK").contentType(MediaType.APPLICATION_JSON)) + } } else { mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) .andExpect(MockRestRequestMatchers.method(HttpMethod.GET)) @@ -77,7 +84,22 @@ class SentrySpanRestTemplateCustomizerTest { fun `when transaction is active and response code is not 2xx, creates span with error status around RestTemplate HTTP call`() { try { fixture.getSut(isTransactionActive = true, status = HttpStatus.INTERNAL_SERVER_ERROR).getForObject("/test/{id}", String::class.java, 123) - } catch (e: Throwable) {} + } catch (e: Throwable) { + } + assertThat(fixture.transaction.spans).hasSize(1) + val span = fixture.transaction.spans.first() + assertThat(span.operation).isEqualTo("http.client") + assertThat(span.description).isEqualTo("GET /test/123") + assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + fixture.mockServer.verify() + } + + @Test + fun `when transaction is active and throws IO exception, creates span with error status around RestTemplate HTTP call`() { + try { + fixture.getSut(isTransactionActive = true, throwIOException = true).getForObject("/test/{id}", String::class.java, 123) + } catch (e: Throwable) { + } assertThat(fixture.transaction.spans).hasSize(1) val span = fixture.transaction.spans.first() assertThat(span.operation).isEqualTo("http.client") diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 7fcb6dfe1a..8364be1e78 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -39,8 +39,14 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); try { final ClientHttpResponse response = execution.execute(request, body); + // handles both success and error responses span.setStatus(SpanStatus.fromHttpStatusCode(response.getRawStatusCode())); return response; + } catch (Exception e) { + // handles cases like connection errors + span.setThrowable(e); + span.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; } finally { span.finish(); }