Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ module.exports = [
path: 'packages/browser/build/npm/esm/prod/index.js',
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
gzip: true,
limit: '85.5 KB',
limit: '85.55 KB',
},
{
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand Down Expand Up @@ -163,7 +163,7 @@ module.exports = [
path: 'packages/vue/build/esm/index.js',
import: createImport('init', 'browserTracingIntegration'),
gzip: true,
limit: '44 KB',
limit: '44.1 KB',
},
// Svelte SDK (ESM)
{
Expand Down
20 changes: 18 additions & 2 deletions packages/core/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,25 @@ export function createTransport(
const requestTask = (): PromiseLike<TransportMakeRequestResponse> =>
makeRequest({ body: serializeEnvelope(filteredEnvelope) }).then(
response => {
// Handle 413 Content Too Large
// Loss of envelope content is expected so we record a send_error client report
// https://develop.sentry.dev/sdk/expected-features/#dealing-with-network-failures
if (response.statusCode === 413) {
DEBUG_BUILD &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super-l: Suggestion for a follow-up: Do you think we could save some bytes here by combining the two debug warns (the one added here and the one below)? I think we could even guard the entire if block in line 87 with DEBUG_BUILD which should shake out some more bytes. We can check that when running size checks in the PR and looking at the non-debug CDN bundles. Feel free to disregard!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check to treeshake the block down there, but could not refactor the message in a way that saves up some byte, since both messages will get dropped if debug is disabled.

debug.error(
'Sentry responded with status code 413. Envelope was discarded due to exceeding size limits.',
);
recordEnvelopeLoss('send_error');
return response;
}

// We don't want to throw on NOK responses, but we want to at least log them
if (response.statusCode !== undefined && (response.statusCode < 200 || response.statusCode >= 300)) {
DEBUG_BUILD && debug.warn(`Sentry responded with status code ${response.statusCode} to sent event.`);
if (
DEBUG_BUILD &&
response.statusCode !== undefined &&
(response.statusCode < 200 || response.statusCode >= 300)
) {
debug.warn(`Sentry responded with status code ${response.statusCode} to sent event.`);
}

rateLimits = updateRateLimits(rateLimits, response);
Expand Down
76 changes: 76 additions & 0 deletions packages/core/test/lib/transports/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,5 +391,81 @@ describe('createTransport', () => {
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error');
});
});

describe('HTTP 413 Content Too Large', () => {
it('should record send_error outcome when receiving 413 response', async () => {
const mockRecordDroppedEventCallback = vi.fn();

const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
resolvedSyncPromise({ statusCode: 413 }),
);

const result = await transport.send(ERROR_ENVELOPE);

// Should resolve without throwing
expect(result).toEqual({ statusCode: 413 });
// recordDroppedEvent SHOULD be called with send_error reason
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
});

it('should record send_error for each item in envelope when receiving 413', async () => {
const mockRecordDroppedEventCallback = vi.fn();

const multiItemEnvelope = createEnvelope<EventEnvelope>(
{ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' },
[
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
[{ type: 'transaction' }, { event_id: 'bb3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
],
);

const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
resolvedSyncPromise({ statusCode: 413 }),
);

await transport.send(multiItemEnvelope);

// recordDroppedEvent SHOULD be called for each item
expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(2);
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'transaction');
});

it('should not record outcomes for client reports when receiving 413', async () => {
const mockRecordDroppedEventCallback = vi.fn();

const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () =>
resolvedSyncPromise({ statusCode: 413 }),
);

const result = await transport.send(CLIENT_REPORT_ENVELOPE);

// Should resolve without throwing
expect(result).toEqual({ statusCode: 413 });
// recordDroppedEvent should NOT be called for client reports to avoid feedback loop
expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled();
});

it('should not apply rate limits after receiving 413', async () => {
const mockRecordDroppedEventCallback = vi.fn();
const mockRequestExecutor = vi.fn(() => resolvedSyncPromise({ statusCode: 413 }));

const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, mockRequestExecutor);

// First request gets 413
await transport.send(ERROR_ENVELOPE);
expect(mockRequestExecutor).toHaveBeenCalledTimes(1);
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error');
mockRequestExecutor.mockClear();
mockRecordDroppedEventCallback.mockClear();

// Second request should still be sent (no rate limiting applied from 413)
mockRequestExecutor.mockImplementation(() => resolvedSyncPromise({}));
await transport.send(ERROR_ENVELOPE);
expect(mockRequestExecutor).toHaveBeenCalledTimes(1);
// No send_error recorded for successful request
expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled();
});
});
});
});
Loading