Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,16 @@ public void captureEnvelope(String rawBytes, ReadableMap options, Promise promis
try {
InternalSentrySdk.captureEnvelope(
bytes, !options.hasKey("hardCrashed") || !options.getBoolean("hardCrashed"));

// TODO(alwx): Capture transport response from sentry-android when available
Copy link
Collaborator

Choose a reason for hiding this comment

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

I could be wrong, but I think the flush/transport is async so the http error will not be triggered here.

WritableMap response = new WritableNativeMap();
promise.resolve(response);
} catch (Throwable e) { // NOPMD - We don't want to crash in any case
logger.log(SentryLevel.ERROR, "Error while capturing envelope");
promise.resolve(false);
logger.log(SentryLevel.ERROR, "Error while capturing envelope", e);
WritableMap errorResponse = new WritableNativeMap();
errorResponse.putString("message", "Error while capturing envelope: " + e.getMessage());
promise.resolve(errorResponse);
}
promise.resolve(true);
}

public void captureScreenshot(Promise promise) {
Expand Down
4 changes: 3 additions & 1 deletion packages/core/ios/RNSentry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,9 @@ - (void)stopObserving
[PrivateSentrySDKOnly captureEnvelope:envelope];
}
#endif
resolve(@YES);

// TODO(alwx): Capture transport response from sentry-cocoa when available
Copy link
Contributor

Choose a reason for hiding this comment

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

The iOS implementation may arrive soon in a next Cocoa 9.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could add it once it's there, shouldn't be too urgent anyway

Copy link
Member

Choose a reason for hiding this comment

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

We released this yesterday with 9.3.0. But the transport doesn't expose the response code. It simply logs an error. Don't these Cocoa SDK logs don't show up in your RN logs? If yes, then all good nothing to do. If not, we need to align on how we can communicate this. This won't be straight forward, because we first store envelopes to disk and then send these async.

resolve(@ {});
}

RCT_EXPORT_METHOD(
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/js/NativeRNSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface Spec extends TurboModule {
options: {
hardCrashed: boolean;
},
): Promise<boolean>;
): Promise<{ status?: string; message?: string }>;
captureScreenshot(): Promise<NativeScreenshot[] | undefined | null>;
clearBreadcrumbs(): void;
crash(): void;
Expand Down
21 changes: 20 additions & 1 deletion packages/core/src/js/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,26 @@ export const NATIVE: SentryNativeWrapper = {
envelopeBytes = newBytes;
}

await RNSentry.captureEnvelope(base64StringFromByteArray(envelopeBytes), { hardCrashed });
const response = await RNSentry.captureEnvelope(base64StringFromByteArray(envelopeBytes), { hardCrashed });

if (response?.status) {
const statusCode = parseInt(response.status, 10);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for the PR!, the solution is nice, but I fear just parsing an integer from a string may be flaky when other types of error happen. Would it be possible to return the statusCode as integer and the errormessage on separated parameters on the native side?


// Handle HTTP 413 - Content Too Large
if (statusCode === 413) {
debug.error(
'Failed to send event to Sentry: HTTP 413 - Envelope exceeds size limit.\n' +
'The event was rejected because the envelope size exceeds the maximum allowed size.\n' +
'Consider reducing the size of breadcrumbs, context data, or attachments.',
);
}
// Log other error status codes
else if (statusCode >= 400) {
debug.error(
`Failed to send event to Sentry: HTTP ${statusCode}${response.message ? ` - ${response.message}` : ''}`,
);
}
}
},

/**
Expand Down
55 changes: 55 additions & 0 deletions packages/core/test/wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,61 @@ describe('Tests Native Wrapper', () => {
{ hardCrashed: false },
);
});

test('handles HTTP 413 response from native', async () => {
const errorSpy = jest.spyOn(debug, 'error');

// Mock native module to return HTTP 413
(RNSentry.captureEnvelope as jest.Mock).mockResolvedValueOnce({
status: '413',
message: 'Payload Too Large',
});

const event = {
event_id: 'event0',
message: 'test',
};

const env = createEnvelope<EventEnvelope>({ event_id: event.event_id, sent_at: '123' }, [
[{ type: 'event' }, event] as EventItem,
]);

await NATIVE.sendEnvelope(env);

expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('HTTP 413 - Envelope exceeds size limit'));
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('Consider reducing the size of breadcrumbs'));

errorSpy.mockRestore();
// Reset mock to default behavior
(RNSentry.captureEnvelope as jest.Mock).mockResolvedValue({});
});

test('handles other HTTP error responses from native', async () => {
const errorSpy = jest.spyOn(debug, 'error');

// Mock native module to return HTTP 400
(RNSentry.captureEnvelope as jest.Mock).mockResolvedValueOnce({
status: '400',
message: 'Bad Request',
});

const event = {
event_id: 'event0',
message: 'test',
};

const env = createEnvelope<EventEnvelope>({ event_id: event.event_id, sent_at: '123' }, [
[{ type: 'event' }, event] as EventItem,
]);

await NATIVE.sendEnvelope(env);

expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('HTTP 400 - Bad Request'));

errorSpy.mockRestore();
// Reset mock to default behavior
(RNSentry.captureEnvelope as jest.Mock).mockResolvedValue({});
});
});

describe('fetchRelease', () => {
Expand Down
Loading