Skip to content

Commit 7ade71d

Browse files
committed
fix(core): Record client report with reason for HTTP 413 responses
1 parent c61baee commit 7ade71d

File tree

3 files changed

+89
-1
lines changed

3 files changed

+89
-1
lines changed

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ module.exports = [
8282
path: 'packages/browser/build/npm/esm/prod/index.js',
8383
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
8484
gzip: true,
85-
limit: '85.5 KB',
85+
limit: '85.55 KB',
8686
},
8787
{
8888
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',

packages/core/src/transports/base.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ export function createTransport(
7171
const requestTask = (): PromiseLike<TransportMakeRequestResponse> =>
7272
makeRequest({ body: serializeEnvelope(filteredEnvelope) }).then(
7373
response => {
74+
// Handle 413 Content Too Large
75+
// Loss of envelope content is expected so we record a send_error client report
76+
// https://develop.sentry.dev/sdk/expected-features/#dealing-with-network-failures
77+
if (response.statusCode === 413) {
78+
DEBUG_BUILD &&
79+
debug.error(
80+
'Sentry responded with status code 413. Envelope was discarded due to exceeding size limits.',
81+
);
82+
recordEnvelopeLoss('send_error');
83+
return response;
84+
}
85+
7486
// We don't want to throw on NOK responses, but we want to at least log them
7587
if (response.statusCode !== undefined && (response.statusCode < 200 || response.statusCode >= 300)) {
7688
DEBUG_BUILD && debug.warn(`Sentry responded with status code ${response.statusCode} to sent event.`);

packages/core/test/lib/transports/base.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,5 +391,81 @@ describe('createTransport', () => {
391391
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error');
392392
});
393393
});
394+
395+
describe('HTTP 413 Content Too Large', () => {
396+
it('should record send_error outcome when receiving 413 response', async () => {
397+
const mockRecordDroppedEventCallback = vi.fn();
398+
399+
const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
400+
resolvedSyncPromise({ statusCode: 413 }),
401+
);
402+
403+
const result = await transport.send(ERROR_ENVELOPE);
404+
405+
// Should resolve without throwing
406+
expect(result).toEqual({ statusCode: 413 });
407+
// recordDroppedEvent SHOULD be called with send_error reason
408+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
409+
});
410+
411+
it('should record send_error for each item in envelope when receiving 413', async () => {
412+
const mockRecordDroppedEventCallback = vi.fn();
413+
414+
const multiItemEnvelope = createEnvelope<EventEnvelope>(
415+
{ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' },
416+
[
417+
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
418+
[{ type: 'transaction' }, { event_id: 'bb3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
419+
],
420+
);
421+
422+
const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
423+
resolvedSyncPromise({ statusCode: 413 }),
424+
);
425+
426+
await transport.send(multiItemEnvelope);
427+
428+
// recordDroppedEvent SHOULD be called for each item
429+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(2);
430+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
431+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'transaction');
432+
});
433+
434+
it('should not record outcomes for client reports when receiving 413', async () => {
435+
const mockRecordDroppedEventCallback = vi.fn();
436+
437+
const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
438+
resolvedSyncPromise({ statusCode: 413 }),
439+
);
440+
441+
const result = await transport.send(CLIENT_REPORT_ENVELOPE);
442+
443+
// Should resolve without throwing
444+
expect(result).toEqual({ statusCode: 413 });
445+
// recordDroppedEvent should NOT be called for client reports to avoid feedback loop
446+
expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled();
447+
});
448+
449+
it('should not apply rate limits after receiving 413', async () => {
450+
const mockRecordDroppedEventCallback = vi.fn();
451+
const mockRequestExecutor = vi.fn(() => resolvedSyncPromise({ statusCode: 413 }));
452+
453+
const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, mockRequestExecutor);
454+
455+
// First request gets 413
456+
await transport.send(ERROR_ENVELOPE);
457+
expect(mockRequestExecutor).toHaveBeenCalledTimes(1);
458+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
459+
mockRequestExecutor.mockClear();
460+
mockRecordDroppedEventCallback.mockClear();
461+
462+
// Second request should still be sent (no rate limiting applied from 413)
463+
mockRequestExecutor.mockImplementation(() => resolvedSyncPromise({}));
464+
await transport.send(ERROR_ENVELOPE);
465+
expect(mockRequestExecutor).toHaveBeenCalledTimes(1);
466+
// No send_error recorded for successful request
467+
expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled();
468+
});
469+
});
394470
});
395471
});

0 commit comments

Comments
 (0)