fix(transport): Handle HTTP 413 with actionable log and use send_error for HTTP errors

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Fix crash when unregistering `SystemEventsBroadcastReceiver` with try-catch block. ([#5106](https://github.com/getsentry/sentry-java/pull/5106))
+- Log an actionable error message when Relay returns HTTP 413 (Content Too Large) ([#5115](https://github.com/getsentry/sentry-java/pull/5115))
 
 ## 8.33.0
 

sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java

@@ -113,16 +113,29 @@ public void send(final @NotNull SentryEnvelope envelope, final @NotNull Hint hin
                   @Override
                   public void completed(SimpleHttpResponse response) {
                     if (response.getCode() != 200) {
-                      options
-                          .getLogger()
-                          .log(ERROR, "Request failed, API returned %s", response.getCode());
+                      if (response.getCode() == 413) {
+                        options
+                            .getLogger()
+                            .log(
+                                ERROR,
+                                "Envelope was discarded by the server because it was too large."
+                                    + " Consider reducing the size of events, breadcrumbs,"
+                                    + " or attachments."
+                                    + " You can use the `SentryOptions.onOversizedEvent`"
+                                    + " callback to customize how oversized events"
+                                    + " are handled.");
+                      } else {
+                        options
+                            .getLogger()
+                            .log(ERROR, "Request failed, API returned %s", response.getCode());
+                      }
 
                       if (response.getCode() >= 400 && response.getCode() != 429) {
                         if (!HintUtils.hasType(hint, Retryable.class)) {
                           options
                               .getClientReportRecorder()
                               .recordLostEnvelope(
-                                  DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
+                                  DiscardReason.SEND_ERROR, envelopeWithClientReport);
                         }
                       }
                     } else {

sentry-apache-http-client-5/src/test/kotlin/io/sentry/transport/apache/ApacheHttpClientTransportClientReportTest.kt

@@ -145,7 +145,7 @@ class ApacheHttpClientTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterClientReportAttached),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -173,7 +173,7 @@ class ApacheHttpClientTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterClientReportAttached),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -191,6 +191,34 @@ class ApacheHttpClientTransportClientReportTest {
     verifyNoMoreInteractions(fixture.clientReportRecorder)
   }
 
+  @Test
+  fun `records lost envelope with send_error on 413 for non retryable`() {
+    val sut = fixture.getSut(SimpleHttpResponse(413))
+
+    sut.send(fixture.envelopeBeforeClientReportAttached)
+
+    verify(fixture.clientReportRecorder, times(1))
+      .attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
+    verify(fixture.clientReportRecorder, times(1))
+      .recordLostEnvelope(
+        eq(DiscardReason.SEND_ERROR),
+        same(fixture.envelopeAfterClientReportAttached),
+      )
+    verifyNoMoreInteractions(fixture.clientReportRecorder)
+  }
+
+  @Test
+  fun `does not record lost envelope on 413 error for retryable`() {
+    val sut = fixture.getSut(SimpleHttpResponse(413))
+
+    sut.send(fixture.envelopeBeforeClientReportAttached, retryableHint())
+
+    verify(fixture.clientReportRecorder, times(1))
+      .attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
+    verify(fixture.clientReportRecorder, never()).recordLostEnvelope(any(), any())
+    verifyNoMoreInteractions(fixture.clientReportRecorder)
+  }
+
   @Test
   fun `does not record lost envelope on 429 error for non retryable`() {
     val sut = fixture.getSut(SimpleHttpResponse(429))

sentry/api/sentry.api

@@ -4793,6 +4793,7 @@ public final class io/sentry/clientreport/DiscardReason : java/lang/Enum {
 	public static final field QUEUE_OVERFLOW Lio/sentry/clientreport/DiscardReason;
 	public static final field RATELIMIT_BACKOFF Lio/sentry/clientreport/DiscardReason;
 	public static final field SAMPLE_RATE Lio/sentry/clientreport/DiscardReason;
+	public static final field SEND_ERROR Lio/sentry/clientreport/DiscardReason;
 	public fun getReason ()Ljava/lang/String;
 	public static fun valueOf (Ljava/lang/String;)Lio/sentry/clientreport/DiscardReason;
 	public static fun values ()[Lio/sentry/clientreport/DiscardReason;

sentry/src/main/java/io/sentry/clientreport/DiscardReason.java

@@ -5,6 +5,7 @@ public enum DiscardReason {
   CACHE_OVERFLOW("cache_overflow"),
   RATELIMIT_BACKOFF("ratelimit_backoff"),
   NETWORK_ERROR("network_error"),
+  SEND_ERROR("send_error"),
   SAMPLE_RATE("sample_rate"),
   BEFORE_SEND("before_send"),
   EVENT_PROCESSOR("event_processor"), // also for ignored exceptions

sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java

@@ -303,9 +303,18 @@ public void run() {
           if (result.isSuccess()) {
             envelopeCache.discard(envelope);
           } else {
-            final String message =
-                "The transport failed to send the envelope with response code "
-                    + result.getResponseCode();
+            final String message;
+            if (result.getResponseCode() == 413) {
+              message =
+                  "Envelope was discarded by the server because it was too large."
+                      + " Consider reducing the size of events, breadcrumbs, or attachments."
+                      + " You can use the `SentryOptions.onOversizedEvent` callback"
+                      + " to customize how oversized events are handled.";
+            } else {
+              message =
+                  "The transport failed to send the envelope with response code "
+                      + result.getResponseCode();
+            }
 
             options.getLogger().log(SentryLevel.ERROR, message);
 
@@ -315,7 +324,7 @@ public void run() {
               if (result.getResponseCode() != 429) {
                 options
                     .getClientReportRecorder()
-                    .recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
+                    .recordLostEnvelope(DiscardReason.SEND_ERROR, envelopeWithClientReport);
               }
             }
 

sentry/src/main/java/io/sentry/transport/HttpConnection.java

@@ -180,7 +180,18 @@ HttpURLConnection open() throws IOException {
       updateRetryAfterLimits(connection, responseCode);
 
       if (!isSuccessfulResponseCode(responseCode)) {
-        options.getLogger().log(ERROR, "Request failed, API returned %s", responseCode);
+        if (responseCode == 413) {
+          options
+              .getLogger()
+              .log(
+                  ERROR,
+                  "Envelope was discarded by the server because it was too large."
+                      + " Consider reducing the size of events, breadcrumbs, or attachments."
+                      + " You can use the `SentryOptions.onOversizedEvent` callback"
+                      + " to customize how oversized events are handled.");
+        } else {
+          options.getLogger().log(ERROR, "Request failed, API returned %s", responseCode);
+        }
         // double check because call is expensive
         if (options.isDebug()) {
           final @NotNull String errorMessage = getErrorMessageFromStream(connection);

sentry/src/test/java/io/sentry/transport/AsyncHttpTransportClientReportTest.kt

@@ -94,7 +94,7 @@ class AsyncHttpTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterAttachingClientReport),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -118,7 +118,7 @@ class AsyncHttpTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterAttachingClientReport),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -145,7 +145,7 @@ class AsyncHttpTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterAttachingClientReport),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -169,12 +169,63 @@ class AsyncHttpTransportClientReportTest {
       .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
     verify(fixture.clientReportRecorder, times(1))
       .recordLostEnvelope(
-        eq(DiscardReason.NETWORK_ERROR),
+        eq(DiscardReason.SEND_ERROR),
         same(fixture.envelopeAfterAttachingClientReport),
       )
     verifyNoMoreInteractions(fixture.clientReportRecorder)
   }
 
+  @Test
+  fun `records lost envelope with send_error on 413 for retryable`() {
+    // given
+    givenSetup(TransportResult.error(413))
+    whenever(
+        fixture.envelopeCache.storeEnvelope(eq(fixture.envelopeBeforeAttachingClientReport), any())
+      )
+      .thenReturn(true)
+
+    // when
+    val retryableHint = retryableHint()
+    assertFailsWith(java.lang.IllegalStateException::class) {
+      fixture.getSUT().send(fixture.envelopeBeforeAttachingClientReport, retryableHint)
+    }
+
+    // then
+    verify(fixture.clientReportRecorder, times(1))
+      .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
+    verify(fixture.clientReportRecorder, times(1))
+      .recordLostEnvelope(
+        eq(DiscardReason.SEND_ERROR),
+        same(fixture.envelopeAfterAttachingClientReport),
+      )
+    verifyNoMoreInteractions(fixture.clientReportRecorder)
+    val sentrySdkHint = HintUtils.getSentrySdkHint(retryableHint)
+    assertFalse((sentrySdkHint as Retryable).isRetry)
+    verify(fixture.envelopeCache).discard(fixture.envelopeBeforeAttachingClientReport)
+  }
+
+  @Test
+  fun `records lost envelope with send_error on 413 for non retryable`() {
+    // given
+    givenSetup(TransportResult.error(413))
+
+    // when
+    assertFailsWith(java.lang.IllegalStateException::class) {
+      fixture.getSUT().send(fixture.envelopeBeforeAttachingClientReport)
+    }
+
+    // then
+    verify(fixture.clientReportRecorder, times(1))
+      .attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
+    verify(fixture.clientReportRecorder, times(1))
+      .recordLostEnvelope(
+        eq(DiscardReason.SEND_ERROR),
+        same(fixture.envelopeAfterAttachingClientReport),
+      )
+    verifyNoMoreInteractions(fixture.clientReportRecorder)
+    verify(fixture.envelopeCache).discard(fixture.envelopeBeforeAttachingClientReport)
+  }
+
   @Test
   fun `records lost envelope on full queue for non retryable`() {
     // given

sentry/src/test/java/io/sentry/transport/AsyncHttpTransportTest.kt

@@ -156,6 +156,31 @@ class AsyncHttpTransportTest {
     order.verify(fixture.sentryOptions.envelopeDiskCache).discard(eq(envelope))
   }
 
+  @Test
+  fun `discards envelope after unsuccessful send 413`() {
+    // given
+    val envelope = SentryEnvelope.from(fixture.sentryOptions.serializer, createSession(), null)
+    whenever(fixture.transportGate.isConnected).thenReturn(true)
+    whenever(fixture.rateLimiter.filter(eq(envelope), anyOrNull())).thenReturn(envelope)
+    whenever(fixture.connection.send(any())).thenReturn(TransportResult.error(413))
+
+    // when
+    try {
+      fixture.getSUT().send(envelope)
+    } catch (e: IllegalStateException) {
+      // expected - this is how the AsyncConnection signals failure to the executor for it to retry
+    }
+
+    // then
+    val order = inOrder(fixture.connection, fixture.sentryOptions.envelopeDiskCache)
+
+    // because storeBeforeSend is enabled by default
+    order.verify(fixture.sentryOptions.envelopeDiskCache).storeEnvelope(eq(envelope), anyOrNull())
+
+    order.verify(fixture.connection).send(eq(envelope))
+    order.verify(fixture.sentryOptions.envelopeDiskCache).discard(eq(envelope))
+  }
+
   @Test
   fun `discards envelope after unsuccessful send 429`() {
     // given