Skip to content

Commit 07ea003

Browse files
adinauerclaude
adinauer
and
claude
committed
fix(transport): Handle HTTP 413 with actionable log and use send_error for HTTP errors
Log a specific, actionable error message when Relay returns HTTP 413 (Content Too Large) instead of the generic "Request failed" message. The message suggests reducing event/breadcrumb/attachment sizes and mentions the `SentryOptions.onOversizedEvent` callback. Also switch the client report discard reason for all HTTP 4xx/5xx errors (except 429) from `network_error` to `send_error`, matching the client reports spec and aligning with sentry-python and sentry-cocoa. Fixes GH-5050 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 70118e9 commit 07ea003

File tree

8 files changed

+154
-15
lines changed

8 files changed

+154
-15
lines changed

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,29 @@ public void send(final @NotNull SentryEnvelope envelope, final @NotNull Hint hin
113113
@Override
114114
public void completed(SimpleHttpResponse response) {
115115
if (response.getCode() != 200) {
116-
options
117-
.getLogger()
118-
.log(ERROR, "Request failed, API returned %s", response.getCode());
116+
if (response.getCode() == 413) {
117+
options
118+
.getLogger()
119+
.log(
120+
ERROR,
121+
"Envelope was discarded by the server because it was too large."
122+
+ " Consider reducing the size of events, breadcrumbs,"
123+
+ " or attachments."
124+
+ " You can use the `SentryOptions.onOversizedEvent`"
125+
+ " callback to customize how oversized events"
126+
+ " are handled.");
127+
} else {
128+
options
129+
.getLogger()
130+
.log(ERROR, "Request failed, API returned %s", response.getCode());
131+
}
119132

120133
if (response.getCode() >= 400 && response.getCode() != 429) {
121134
if (!HintUtils.hasType(hint, Retryable.class)) {
122135
options
123136
.getClientReportRecorder()
124137
.recordLostEnvelope(
125-
DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
138+
DiscardReason.SEND_ERROR, envelopeWithClientReport);
126139
}
127140
}
128141
} else {

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class ApacheHttpClientTransportClientReportTest {
145145
.attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
146146
verify(fixture.clientReportRecorder, times(1))
147147
.recordLostEnvelope(
148-
eq(DiscardReason.NETWORK_ERROR),
148+
eq(DiscardReason.SEND_ERROR),
149149
same(fixture.envelopeAfterClientReportAttached),
150150
)
151151
verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -173,7 +173,7 @@ class ApacheHttpClientTransportClientReportTest {
173173
.attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
174174
verify(fixture.clientReportRecorder, times(1))
175175
.recordLostEnvelope(
176-
eq(DiscardReason.NETWORK_ERROR),
176+
eq(DiscardReason.SEND_ERROR),
177177
same(fixture.envelopeAfterClientReportAttached),
178178
)
179179
verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -191,6 +191,34 @@ class ApacheHttpClientTransportClientReportTest {
191191
verifyNoMoreInteractions(fixture.clientReportRecorder)
192192
}
193193

194+
@Test
195+
fun `records lost envelope with send_error on 413 for non retryable`() {
196+
val sut = fixture.getSut(SimpleHttpResponse(413))
197+
198+
sut.send(fixture.envelopeBeforeClientReportAttached)
199+
200+
verify(fixture.clientReportRecorder, times(1))
201+
.attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
202+
verify(fixture.clientReportRecorder, times(1))
203+
.recordLostEnvelope(
204+
eq(DiscardReason.SEND_ERROR),
205+
same(fixture.envelopeAfterClientReportAttached),
206+
)
207+
verifyNoMoreInteractions(fixture.clientReportRecorder)
208+
}
209+
210+
@Test
211+
fun `does not record lost envelope on 413 error for retryable`() {
212+
val sut = fixture.getSut(SimpleHttpResponse(413))
213+
214+
sut.send(fixture.envelopeBeforeClientReportAttached, retryableHint())
215+
216+
verify(fixture.clientReportRecorder, times(1))
217+
.attachReportToEnvelope(same(fixture.envelopeBeforeClientReportAttached))
218+
verify(fixture.clientReportRecorder, never()).recordLostEnvelope(any(), any())
219+
verifyNoMoreInteractions(fixture.clientReportRecorder)
220+
}
221+
194222
@Test
195223
fun `does not record lost envelope on 429 error for non retryable`() {
196224
val sut = fixture.getSut(SimpleHttpResponse(429))

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4793,6 +4793,7 @@ public final class io/sentry/clientreport/DiscardReason : java/lang/Enum {
47934793
public static final field QUEUE_OVERFLOW Lio/sentry/clientreport/DiscardReason;
47944794
public static final field RATELIMIT_BACKOFF Lio/sentry/clientreport/DiscardReason;
47954795
public static final field SAMPLE_RATE Lio/sentry/clientreport/DiscardReason;
4796+
public static final field SEND_ERROR Lio/sentry/clientreport/DiscardReason;
47964797
public fun getReason ()Ljava/lang/String;
47974798
public static fun valueOf (Ljava/lang/String;)Lio/sentry/clientreport/DiscardReason;
47984799
public static fun values ()[Lio/sentry/clientreport/DiscardReason;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ public enum DiscardReason {
55
CACHE_OVERFLOW("cache_overflow"),
66
RATELIMIT_BACKOFF("ratelimit_backoff"),
77
NETWORK_ERROR("network_error"),
8+
SEND_ERROR("send_error"),
89
SAMPLE_RATE("sample_rate"),
910
BEFORE_SEND("before_send"),
1011
EVENT_PROCESSOR("event_processor"), // also for ignored exceptions

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,18 @@ public void run() {
303303
if (result.isSuccess()) {
304304
envelopeCache.discard(envelope);
305305
} else {
306-
final String message =
307-
"The transport failed to send the envelope with response code "
308-
+ result.getResponseCode();
306+
final String message;
307+
if (result.getResponseCode() == 413) {
308+
message =
309+
"Envelope was discarded by the server because it was too large."
310+
+ " Consider reducing the size of events, breadcrumbs, or attachments."
311+
+ " You can use the `SentryOptions.onOversizedEvent` callback"
312+
+ " to customize how oversized events are handled.";
313+
} else {
314+
message =
315+
"The transport failed to send the envelope with response code "
316+
+ result.getResponseCode();
317+
}
309318

310319
options.getLogger().log(SentryLevel.ERROR, message);
311320

@@ -315,7 +324,7 @@ public void run() {
315324
if (result.getResponseCode() != 429) {
316325
options
317326
.getClientReportRecorder()
318-
.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelopeWithClientReport);
327+
.recordLostEnvelope(DiscardReason.SEND_ERROR, envelopeWithClientReport);
319328
}
320329
}
321330

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,18 @@ HttpURLConnection open() throws IOException {
180180
updateRetryAfterLimits(connection, responseCode);
181181

182182
if (!isSuccessfulResponseCode(responseCode)) {
183-
options.getLogger().log(ERROR, "Request failed, API returned %s", responseCode);
183+
if (responseCode == 413) {
184+
options
185+
.getLogger()
186+
.log(
187+
ERROR,
188+
"Envelope was discarded by the server because it was too large."
189+
+ " Consider reducing the size of events, breadcrumbs, or attachments."
190+
+ " You can use the `SentryOptions.onOversizedEvent` callback"
191+
+ " to customize how oversized events are handled.");
192+
} else {
193+
options.getLogger().log(ERROR, "Request failed, API returned %s", responseCode);
194+
}
184195
// double check because call is expensive
185196
if (options.isDebug()) {
186197
final @NotNull String errorMessage = getErrorMessageFromStream(connection);

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

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class AsyncHttpTransportClientReportTest {
9494
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
9595
verify(fixture.clientReportRecorder, times(1))
9696
.recordLostEnvelope(
97-
eq(DiscardReason.NETWORK_ERROR),
97+
eq(DiscardReason.SEND_ERROR),
9898
same(fixture.envelopeAfterAttachingClientReport),
9999
)
100100
verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -118,7 +118,7 @@ class AsyncHttpTransportClientReportTest {
118118
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
119119
verify(fixture.clientReportRecorder, times(1))
120120
.recordLostEnvelope(
121-
eq(DiscardReason.NETWORK_ERROR),
121+
eq(DiscardReason.SEND_ERROR),
122122
same(fixture.envelopeAfterAttachingClientReport),
123123
)
124124
verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -145,7 +145,7 @@ class AsyncHttpTransportClientReportTest {
145145
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
146146
verify(fixture.clientReportRecorder, times(1))
147147
.recordLostEnvelope(
148-
eq(DiscardReason.NETWORK_ERROR),
148+
eq(DiscardReason.SEND_ERROR),
149149
same(fixture.envelopeAfterAttachingClientReport),
150150
)
151151
verifyNoMoreInteractions(fixture.clientReportRecorder)
@@ -169,12 +169,63 @@ class AsyncHttpTransportClientReportTest {
169169
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
170170
verify(fixture.clientReportRecorder, times(1))
171171
.recordLostEnvelope(
172-
eq(DiscardReason.NETWORK_ERROR),
172+
eq(DiscardReason.SEND_ERROR),
173173
same(fixture.envelopeAfterAttachingClientReport),
174174
)
175175
verifyNoMoreInteractions(fixture.clientReportRecorder)
176176
}
177177

178+
@Test
179+
fun `records lost envelope with send_error on 413 for retryable`() {
180+
// given
181+
givenSetup(TransportResult.error(413))
182+
whenever(
183+
fixture.envelopeCache.storeEnvelope(eq(fixture.envelopeBeforeAttachingClientReport), any())
184+
)
185+
.thenReturn(true)
186+
187+
// when
188+
val retryableHint = retryableHint()
189+
assertFailsWith(java.lang.IllegalStateException::class) {
190+
fixture.getSUT().send(fixture.envelopeBeforeAttachingClientReport, retryableHint)
191+
}
192+
193+
// then
194+
verify(fixture.clientReportRecorder, times(1))
195+
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
196+
verify(fixture.clientReportRecorder, times(1))
197+
.recordLostEnvelope(
198+
eq(DiscardReason.SEND_ERROR),
199+
same(fixture.envelopeAfterAttachingClientReport),
200+
)
201+
verifyNoMoreInteractions(fixture.clientReportRecorder)
202+
val sentrySdkHint = HintUtils.getSentrySdkHint(retryableHint)
203+
assertFalse((sentrySdkHint as Retryable).isRetry)
204+
verify(fixture.envelopeCache).discard(fixture.envelopeBeforeAttachingClientReport)
205+
}
206+
207+
@Test
208+
fun `records lost envelope with send_error on 413 for non retryable`() {
209+
// given
210+
givenSetup(TransportResult.error(413))
211+
212+
// when
213+
assertFailsWith(java.lang.IllegalStateException::class) {
214+
fixture.getSUT().send(fixture.envelopeBeforeAttachingClientReport)
215+
}
216+
217+
// then
218+
verify(fixture.clientReportRecorder, times(1))
219+
.attachReportToEnvelope(same(fixture.envelopeBeforeAttachingClientReport))
220+
verify(fixture.clientReportRecorder, times(1))
221+
.recordLostEnvelope(
222+
eq(DiscardReason.SEND_ERROR),
223+
same(fixture.envelopeAfterAttachingClientReport),
224+
)
225+
verifyNoMoreInteractions(fixture.clientReportRecorder)
226+
verify(fixture.envelopeCache).discard(fixture.envelopeBeforeAttachingClientReport)
227+
}
228+
178229
@Test
179230
fun `records lost envelope on full queue for non retryable`() {
180231
// given

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,31 @@ class AsyncHttpTransportTest {
156156
order.verify(fixture.sentryOptions.envelopeDiskCache).discard(eq(envelope))
157157
}
158158

159+
@Test
160+
fun `discards envelope after unsuccessful send 413`() {
161+
// given
162+
val envelope = SentryEnvelope.from(fixture.sentryOptions.serializer, createSession(), null)
163+
whenever(fixture.transportGate.isConnected).thenReturn(true)
164+
whenever(fixture.rateLimiter.filter(eq(envelope), anyOrNull())).thenReturn(envelope)
165+
whenever(fixture.connection.send(any())).thenReturn(TransportResult.error(413))
166+
167+
// when
168+
try {
169+
fixture.getSUT().send(envelope)
170+
} catch (e: IllegalStateException) {
171+
// expected - this is how the AsyncConnection signals failure to the executor for it to retry
172+
}
173+
174+
// then
175+
val order = inOrder(fixture.connection, fixture.sentryOptions.envelopeDiskCache)
176+
177+
// because storeBeforeSend is enabled by default
178+
order.verify(fixture.sentryOptions.envelopeDiskCache).storeEnvelope(eq(envelope), anyOrNull())
179+
180+
order.verify(fixture.connection).send(eq(envelope))
181+
order.verify(fixture.sentryOptions.envelopeDiskCache).discard(eq(envelope))
182+
}
183+
159184
@Test
160185
fun `discards envelope after unsuccessful send 429`() {
161186
// given

0 commit comments

Comments
 (0)