Skip to content

Commit 86b3c69

Browse files
Lms24Lms24
authored
feat(serverless): Mark errors caught in Serverless handlers as unhandled (#8907)
Mark errors caught in * AWS Lambda handler * GCP http, function, cloudEvent handlers as unhandled For more details see #8890
1 parent eb29981 commit 86b3c69

File tree

7 files changed

+97
-29
lines changed

7 files changed

+97
-29
lines changed

‎packages/serverless/src/awslambda.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { performance } from 'perf_hooks';
1414
import { types } from 'util';
1515

1616
import { AWSServices } from './awsservices';
17-
import { serverlessEventProcessor } from './utils';
17+
import { markEventUnhandled, serverlessEventProcessor } from './utils';
1818

1919
export * from '@sentry/node';
2020

@@ -312,11 +312,11 @@ export function wrapHandler<TEvent, TResult>(
312312
if (options.captureAllSettledReasons && Array.isArray(rv) && isPromiseAllSettledResult(rv)) {
313313
const reasons = getRejectedReasons(rv);
314314
reasons.forEach(exception => {
315-
captureException(exception);
315+
captureException(exception, scope => markEventUnhandled(scope));
316316
});
317317
}
318318
} catch (e) {
319-
captureException(e);
319+
captureException(e, scope => markEventUnhandled(scope));
320320
throw e;
321321
} finally {
322322
clearTimeout(timeoutWarningTimer);

‎packages/serverless/src/gcpfunction/cloud_events.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { captureException, flush, getCurrentHub } from '@sentry/node';
22
import { isThenable, logger } from '@sentry/utils';
33

4-
import { domainify, proxyFunction } from '../utils';
4+
import { domainify, markEventUnhandled, proxyFunction } from '../utils';
55
import type { CloudEventFunction, CloudEventFunctionWithCallback, WrapperOptions } from './general';
66

77
export type CloudEventFunctionWrapperOptions = WrapperOptions;
@@ -50,7 +50,7 @@ function _wrapCloudEventFunction(
5050

5151
const newCallback = domainify((...args: unknown[]) => {
5252
if (args[0] !== null && args[0] !== undefined) {
53-
captureException(args[0]);
53+
captureException(args[0], scope => markEventUnhandled(scope));
5454
}
5555
transaction?.finish();
5656

@@ -68,13 +68,13 @@ function _wrapCloudEventFunction(
6868
try {
6969
fnResult = (fn as CloudEventFunctionWithCallback)(context, newCallback);
7070
} catch (err) {
71-
captureException(err);
71+
captureException(err, scope => markEventUnhandled(scope));
7272
throw err;
7373
}
7474

7575
if (isThenable(fnResult)) {
7676
fnResult.then(null, err => {
77-
captureException(err);
77+
captureException(err, scope => markEventUnhandled(scope));
7878
throw err;
7979
});
8080
}

‎packages/serverless/src/gcpfunction/events.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { captureException, flush, getCurrentHub } from '@sentry/node';
22
import { isThenable, logger } from '@sentry/utils';
33

4-
import { domainify, proxyFunction } from '../utils';
4+
import { domainify, markEventUnhandled, proxyFunction } from '../utils';
55
import type { EventFunction, EventFunctionWithCallback, WrapperOptions } from './general';
66

77
export type EventFunctionWrapperOptions = WrapperOptions;
@@ -52,7 +52,7 @@ function _wrapEventFunction<F extends EventFunction | EventFunctionWithCallback>
5252

5353
const newCallback = domainify((...args: unknown[]) => {
5454
if (args[0] !== null && args[0] !== undefined) {
55-
captureException(args[0]);
55+
captureException(args[0], scope => markEventUnhandled(scope));
5656
}
5757
transaction?.finish();
5858

@@ -72,13 +72,13 @@ function _wrapEventFunction<F extends EventFunction | EventFunctionWithCallback>
7272
try {
7373
fnResult = (fn as EventFunctionWithCallback)(data, context, newCallback);
7474
} catch (err) {
75-
captureException(err);
75+
captureException(err, scope => markEventUnhandled(scope));
7676
throw err;
7777
}
7878

7979
if (isThenable(fnResult)) {
8080
fnResult.then(null, err => {
81-
captureException(err);
81+
captureException(err, scope => markEventUnhandled(scope));
8282
throw err;
8383
});
8484
}

‎packages/serverless/src/gcpfunction/http.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { AddRequestDataToEventOptions } from '@sentry/node';
22
import { captureException, flush, getCurrentHub } from '@sentry/node';
33
import { isString, isThenable, logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
44

5-
import { domainify, proxyFunction } from './../utils';
5+
import { domainify, markEventUnhandled, proxyFunction } from './../utils';
66
import type { HttpFunction, WrapperOptions } from './general';
77

88
// TODO (v8 / #5257): Remove this whole old/new business and just use the new stuff
@@ -122,13 +122,13 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
122122
try {
123123
fnResult = fn(req, res);
124124
} catch (err) {
125-
captureException(err);
125+
captureException(err, scope => markEventUnhandled(scope));
126126
throw err;
127127
}
128128

129129
if (isThenable(fnResult)) {
130130
fnResult.then(null, err => {
131-
captureException(err);
131+
captureException(err, scope => markEventUnhandled(scope));
132132
throw err;
133133
});
134134
}

‎packages/serverless/src/utils.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { runWithAsyncContext } from '@sentry/core';
22
import type { Event } from '@sentry/node';
3+
import type { Scope } from '@sentry/types';
34
import { addExceptionMechanism } from '@sentry/utils';
45

56
/**
@@ -55,3 +56,15 @@ export function proxyFunction<A extends any[], R, F extends (...args: A) => R>(
5556

5657
return new Proxy(source, handler);
5758
}
59+
60+
/**
61+
* Marks an event as unhandled by adding a span processor to the passed scope.
62+
*/
63+
export function markEventUnhandled(scope: Scope): Scope {
64+
scope.addEventProcessor(event => {
65+
addExceptionMechanism(event, { handled: false });
66+
return event;
67+
});
68+
69+
return scope;
70+
}

‎packages/serverless/test/awslambda.test.ts‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
22
// eslint-disable-next-line import/no-unresolved
33
import * as SentryNode from '@sentry/node';
4+
import type { Event } from '@sentry/types';
45
// eslint-disable-next-line import/no-unresolved
56
import type { Callback, Handler } from 'aws-lambda';
67

@@ -175,8 +176,8 @@ describe('AWSLambda', () => {
175176
]);
176177
const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337, captureAllSettledReasons: true });
177178
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
178-
expect(SentryNode.captureException).toHaveBeenNthCalledWith(1, error);
179-
expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2);
179+
expect(SentryNode.captureException).toHaveBeenNthCalledWith(1, error, expect.any(Function));
180+
expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function));
180181
expect(SentryNode.captureException).toBeCalledTimes(2);
181182
});
182183
});
@@ -229,7 +230,7 @@ describe('AWSLambda', () => {
229230
// @ts-ignore see "Why @ts-ignore" note
230231
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
231232
expectScopeSettings(fakeTransactionContext);
232-
expect(SentryNode.captureException).toBeCalledWith(error);
233+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
233234
// @ts-ignore see "Why @ts-ignore" note
234235
expect(SentryNode.fakeTransaction.finish).toBeCalled();
235236
expect(SentryNode.flush).toBeCalledWith(2000);
@@ -308,7 +309,7 @@ describe('AWSLambda', () => {
308309
// @ts-ignore see "Why @ts-ignore" note
309310
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
310311
expectScopeSettings(fakeTransactionContext);
311-
expect(SentryNode.captureException).toBeCalledWith(e);
312+
expect(SentryNode.captureException).toBeCalledWith(e, expect.any(Function));
312313
// @ts-ignore see "Why @ts-ignore" note
313314
expect(SentryNode.fakeTransaction.finish).toBeCalled();
314315
expect(SentryNode.flush).toBeCalled();
@@ -375,7 +376,7 @@ describe('AWSLambda', () => {
375376
// @ts-ignore see "Why @ts-ignore" note
376377
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
377378
expectScopeSettings(fakeTransactionContext);
378-
expect(SentryNode.captureException).toBeCalledWith(error);
379+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
379380
// @ts-ignore see "Why @ts-ignore" note
380381
expect(SentryNode.fakeTransaction.finish).toBeCalled();
381382
expect(SentryNode.flush).toBeCalled();
@@ -457,14 +458,42 @@ describe('AWSLambda', () => {
457458
// @ts-ignore see "Why @ts-ignore" note
458459
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
459460
expectScopeSettings(fakeTransactionContext);
460-
expect(SentryNode.captureException).toBeCalledWith(error);
461+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
461462
// @ts-ignore see "Why @ts-ignore" note
462463
expect(SentryNode.fakeTransaction.finish).toBeCalled();
463464
expect(SentryNode.flush).toBeCalled();
464465
}
465466
});
466467
});
467468

469+
test('marks the captured error as unhandled', async () => {
470+
expect.assertions(3);
471+
472+
const error = new Error('wat');
473+
const handler: Handler = async (_event, _context, _callback) => {
474+
throw error;
475+
};
476+
const wrappedHandler = wrapHandler(handler);
477+
478+
try {
479+
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
480+
} catch (e) {
481+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
482+
// @ts-ignore see "Why @ts-ignore" note
483+
const scopeFunction = SentryNode.captureException.mock.calls[0][1];
484+
const event: Event = { exception: { values: [{}] } };
485+
let evtProcessor: ((e: Event) => Event) | undefined = undefined;
486+
scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) });
487+
488+
expect(evtProcessor).toBeInstanceOf(Function);
489+
// @ts-ignore just mocking around...
490+
expect(evtProcessor(event).exception.values[0].mechanism).toEqual({
491+
handled: false,
492+
type: 'generic',
493+
});
494+
}
495+
});
496+
468497
describe('init()', () => {
469498
test('calls Sentry.init with correct sdk info metadata', () => {
470499
Sentry.AWSLambda.init({});

‎packages/serverless/test/gcpfunction.test.ts‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as SentryNode from '@sentry/node';
2+
import type { Event } from '@sentry/types';
23
import * as domain from 'domain';
34

45
import * as Sentry from '../src';
@@ -12,7 +13,6 @@ import type {
1213
Request,
1314
Response,
1415
} from '../src/gcpfunction/general';
15-
1616
/**
1717
* Why @ts-ignore some Sentry.X calls
1818
*
@@ -198,7 +198,7 @@ describe('GCPFunction', () => {
198198
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
199199
// @ts-ignore see "Why @ts-ignore" note
200200
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
201-
expect(SentryNode.captureException).toBeCalledWith(error);
201+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
202202
// @ts-ignore see "Why @ts-ignore" note
203203
expect(SentryNode.fakeTransaction.finish).toBeCalled();
204204
expect(SentryNode.flush).toBeCalled();
@@ -317,7 +317,7 @@ describe('GCPFunction', () => {
317317
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
318318
// @ts-ignore see "Why @ts-ignore" note
319319
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
320-
expect(SentryNode.captureException).toBeCalledWith(error);
320+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
321321
// @ts-ignore see "Why @ts-ignore" note
322322
expect(SentryNode.fakeTransaction.finish).toBeCalled();
323323
expect(SentryNode.flush).toBeCalled();
@@ -382,7 +382,7 @@ describe('GCPFunction', () => {
382382
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
383383
// @ts-ignore see "Why @ts-ignore" note
384384
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
385-
expect(SentryNode.captureException).toBeCalledWith(error);
385+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
386386
// @ts-ignore see "Why @ts-ignore" note
387387
expect(SentryNode.fakeTransaction.finish).toBeCalled();
388388
expect(SentryNode.flush).toBeCalled();
@@ -440,7 +440,7 @@ describe('GCPFunction', () => {
440440
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
441441
// @ts-ignore see "Why @ts-ignore" note
442442
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
443-
expect(SentryNode.captureException).toBeCalledWith(error);
443+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
444444
// @ts-ignore see "Why @ts-ignore" note
445445
expect(SentryNode.fakeTransaction.finish).toBeCalled();
446446
expect(SentryNode.flush).toBeCalled();
@@ -469,7 +469,33 @@ describe('GCPFunction', () => {
469469
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
470470
// @ts-ignore see "Why @ts-ignore" note
471471
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
472-
expect(SentryNode.captureException).toBeCalledWith(error);
472+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
473+
});
474+
});
475+
476+
test('marks the captured error as unhandled', async () => {
477+
expect.assertions(4);
478+
479+
const error = new Error('wat');
480+
const handler: EventFunctionWithCallback = (_data, _context, _cb) => {
481+
throw error;
482+
};
483+
const wrappedHandler = wrapEventFunction(handler);
484+
await expect(handleEvent(wrappedHandler)).rejects.toThrowError(error);
485+
486+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
487+
488+
// @ts-ignore just mocking around...
489+
const scopeFunction = SentryNode.captureException.mock.calls[0][1];
490+
const event: Event = { exception: { values: [{}] } };
491+
let evtProcessor: ((e: Event) => Event) | undefined = undefined;
492+
scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) });
493+
494+
expect(evtProcessor).toBeInstanceOf(Function);
495+
// @ts-ignore just mocking around...
496+
expect(evtProcessor(event).exception.values[0].mechanism).toEqual({
497+
handled: false,
498+
type: 'generic',
473499
});
474500
});
475501

@@ -537,7 +563,7 @@ describe('GCPFunction', () => {
537563
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
538564
// @ts-ignore see "Why @ts-ignore" note
539565
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
540-
expect(SentryNode.captureException).toBeCalledWith(error);
566+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
541567
// @ts-ignore see "Why @ts-ignore" note
542568
expect(SentryNode.fakeTransaction.finish).toBeCalled();
543569
expect(SentryNode.flush).toBeCalled();
@@ -595,7 +621,7 @@ describe('GCPFunction', () => {
595621
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
596622
// @ts-ignore see "Why @ts-ignore" note
597623
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
598-
expect(SentryNode.captureException).toBeCalledWith(error);
624+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
599625
// @ts-ignore see "Why @ts-ignore" note
600626
expect(SentryNode.fakeTransaction.finish).toBeCalled();
601627
expect(SentryNode.flush).toBeCalled();
@@ -625,7 +651,7 @@ describe('GCPFunction', () => {
625651
// @ts-ignore see "Why @ts-ignore" note
626652
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
627653

628-
expect(SentryNode.captureException).toBeCalledWith(error);
654+
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
629655
});
630656
});
631657

0 commit comments

Comments
 (0)