Skip to content

Commit dd17203

Browse files
authored
fix(core): Fix error handling when sending envelopes (#17662)
I noticed that we actually handled errors in `sendEnvelope` incorrectly - we would resolve this function with the rejection reason, if sending fails. this does not match the type of `TransportMakeRequestResponse`, you could actually get something like this out (and the tests actually incorrectly tested this): ```js // transport.send() rejects with "fetch does not exist" const res = await client.sendEnvelope(envelope); // res --> "fetch does not exist" (string) ``` This PR fixes this to instead resolve with an empty object (which matches the expected return type). Extracted this out of #17641 because it is actually a bug/fix.
1 parent e8d255f commit dd17203

File tree

3 files changed

+7
-10
lines changed

3 files changed

+7
-10
lines changed

packages/core/src/client.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,10 +415,9 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
415415
env = addItemToEnvelope(env, createAttachmentEnvelopeItem(attachment));
416416
}
417417

418-
const promise = this.sendEnvelope(env);
419-
if (promise) {
420-
promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null);
421-
}
418+
// sendEnvelope should not throw
419+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
420+
this.sendEnvelope(env).then(sendResponse => this.emit('afterSendEvent', event, sendResponse));
422421
}
423422

424423
/**
@@ -879,12 +878,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
879878
if (this._isEnabled() && this._transport) {
880879
return this._transport.send(envelope).then(null, reason => {
881880
DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
882-
return reason;
881+
return {};
883882
});
884883
}
885884

886885
DEBUG_BUILD && debug.error('Transport disabled');
887-
888886
return resolvedSyncPromise({});
889887
}
890888

packages/core/test/lib/client.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2268,7 +2268,7 @@ describe('Client', () => {
22682268

22692269
expect(mockSend).toBeCalledTimes(1);
22702270
expect(callback).toBeCalledTimes(1);
2271-
expect(callback).toBeCalledWith(errorEvent, 'send error');
2271+
expect(callback).toBeCalledWith(errorEvent, {});
22722272
});
22732273

22742274
it('passes the response to the hook', async () => {

packages/replay-internal/src/coreHandlers/handleAfterSendEvent.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,10 @@ export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCal
1414
return;
1515
}
1616

17-
const statusCode = sendResponse?.statusCode;
17+
const statusCode = sendResponse.statusCode;
1818

1919
// We only want to do stuff on successful error sending, otherwise you get error replays without errors attached
20-
// If not using the base transport, we allow `undefined` response (as a custom transport may not implement this correctly yet)
21-
// If we do use the base transport, we skip if we encountered an non-OK status code
20+
// We skip if we encountered an non-OK status code
2221
if (!statusCode || statusCode < 200 || statusCode >= 300) {
2322
return;
2423
}

0 commit comments

Comments
 (0)