Skip to content

Commit

Permalink
feat(serverless): Mark errors caught in Serverless handlers as unhand…
Browse files Browse the repository at this point in the history 
…led (#8907)

Mark errors caught in

* AWS Lambda handler
* GCP http, function, cloudEvent handlers

as unhandled

For more details see #8890
  • Loading branch information
@Lms24
Lms24 authored Sep 4, 2023
1 parent eb29981 commit 86b3c69
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 29 deletions.
6 changes: 3 additions & 3 deletions packages/serverless/src/awslambda.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { performance } from 'perf_hooks';
import { types } from 'util';

import { AWSServices } from './awsservices';
import { serverlessEventProcessor } from './utils';
import { markEventUnhandled, serverlessEventProcessor } from './utils';

export * from '@sentry/node';

Expand Down Expand Up @@ -312,11 +312,11 @@ export function wrapHandler<TEvent, TResult>(
if (options.captureAllSettledReasons && Array.isArray(rv) && isPromiseAllSettledResult(rv)) {
const reasons = getRejectedReasons(rv);
reasons.forEach(exception => {
captureException(exception);
captureException(exception, scope => markEventUnhandled(scope));
});
}
} catch (e) {
captureException(e);
captureException(e, scope => markEventUnhandled(scope));
throw e;
} finally {
clearTimeout(timeoutWarningTimer);
Expand Down
8 changes: 4 additions & 4 deletions packages/serverless/src/gcpfunction/cloud_events.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { isThenable, logger } from '@sentry/utils';

import { domainify, proxyFunction } from '../utils';
import { domainify, markEventUnhandled, proxyFunction } from '../utils';
import type { CloudEventFunction, CloudEventFunctionWithCallback, WrapperOptions } from './general';

export type CloudEventFunctionWrapperOptions = WrapperOptions;
Expand Down Expand Up @@ -50,7 +50,7 @@ function _wrapCloudEventFunction(

const newCallback = domainify((...args: unknown[]) => {
if (args[0] !== null && args[0] !== undefined) {
captureException(args[0]);
captureException(args[0], scope => markEventUnhandled(scope));
}
transaction?.finish();

Expand All @@ -68,13 +68,13 @@ function _wrapCloudEventFunction(
try {
fnResult = (fn as CloudEventFunctionWithCallback)(context, newCallback);
} catch (err) {
captureException(err);
captureException(err, scope => markEventUnhandled(scope));
throw err;
}

if (isThenable(fnResult)) {
fnResult.then(null, err => {
captureException(err);
captureException(err, scope => markEventUnhandled(scope));
throw err;
});
}
Expand Down
8 changes: 4 additions & 4 deletions packages/serverless/src/gcpfunction/events.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { isThenable, logger } from '@sentry/utils';

import { domainify, proxyFunction } from '../utils';
import { domainify, markEventUnhandled, proxyFunction } from '../utils';
import type { EventFunction, EventFunctionWithCallback, WrapperOptions } from './general';

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

const newCallback = domainify((...args: unknown[]) => {
if (args[0] !== null && args[0] !== undefined) {
captureException(args[0]);
captureException(args[0], scope => markEventUnhandled(scope));
}
transaction?.finish();

Expand All @@ -72,13 +72,13 @@ function _wrapEventFunction<F extends EventFunction | EventFunctionWithCallback>
try {
fnResult = (fn as EventFunctionWithCallback)(data, context, newCallback);
} catch (err) {
captureException(err);
captureException(err, scope => markEventUnhandled(scope));
throw err;
}

if (isThenable(fnResult)) {
fnResult.then(null, err => {
captureException(err);
captureException(err, scope => markEventUnhandled(scope));
throw err;
});
}
Expand Down
6 changes: 3 additions & 3 deletions packages/serverless/src/gcpfunction/http.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { AddRequestDataToEventOptions } from '@sentry/node';
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { isString, isThenable, logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';

import { domainify, proxyFunction } from './../utils';
import { domainify, markEventUnhandled, proxyFunction } from './../utils';
import type { HttpFunction, WrapperOptions } from './general';

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

if (isThenable(fnResult)) {
fnResult.then(null, err => {
captureException(err);
captureException(err, scope => markEventUnhandled(scope));
throw err;
});
}
Expand Down
13 changes: 13 additions & 0 deletions packages/serverless/src/utils.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { runWithAsyncContext } from '@sentry/core';
import type { Event } from '@sentry/node';
import type { Scope } from '@sentry/types';
import { addExceptionMechanism } from '@sentry/utils';

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

return new Proxy(source, handler);
}

/**
* Marks an event as unhandled by adding a span processor to the passed scope.
*/
export function markEventUnhandled(scope: Scope): Scope {
scope.addEventProcessor(event => {
addExceptionMechanism(event, { handled: false });
return event;
});

return scope;
}
41 changes: 35 additions & 6 deletions packages/serverless/test/awslambda.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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
// eslint-disable-next-line import/no-unresolved
import * as SentryNode from '@sentry/node';
import type { Event } from '@sentry/types';
// eslint-disable-next-line import/no-unresolved
import type { Callback, Handler } from 'aws-lambda';

Expand Down Expand Up @@ -175,8 +176,8 @@ describe('AWSLambda', () => {
]);
const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337, captureAllSettledReasons: true });
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
expect(SentryNode.captureException).toHaveBeenNthCalledWith(1, error);
expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2);
expect(SentryNode.captureException).toHaveBeenNthCalledWith(1, error, expect.any(Function));
expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function));
expect(SentryNode.captureException).toBeCalledTimes(2);
});
});
Expand Down Expand Up @@ -229,7 +230,7 @@ describe('AWSLambda', () => {
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalledWith(2000);
Expand Down Expand Up @@ -308,7 +309,7 @@ describe('AWSLambda', () => {
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(SentryNode.captureException).toBeCalledWith(e);
expect(SentryNode.captureException).toBeCalledWith(e, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -375,7 +376,7 @@ describe('AWSLambda', () => {
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -457,14 +458,42 @@ describe('AWSLambda', () => {
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
}
});
});

test('marks the captured error as unhandled', async () => {
expect.assertions(3);

const error = new Error('wat');
const handler: Handler = async (_event, _context, _callback) => {
throw error;
};
const wrappedHandler = wrapHandler(handler);

try {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
} catch (e) {
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
const scopeFunction = SentryNode.captureException.mock.calls[0][1];
const event: Event = { exception: { values: [{}] } };
let evtProcessor: ((e: Event) => Event) | undefined = undefined;
scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) });

expect(evtProcessor).toBeInstanceOf(Function);
// @ts-ignore just mocking around...
expect(evtProcessor(event).exception.values[0].mechanism).toEqual({
handled: false,
type: 'generic',
});
}
});

describe('init()', () => {
test('calls Sentry.init with correct sdk info metadata', () => {
Sentry.AWSLambda.init({});
Expand Down
44 changes: 35 additions & 9 deletions packages/serverless/test/gcpfunction.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as SentryNode from '@sentry/node';
import type { Event } from '@sentry/types';
import * as domain from 'domain';

import * as Sentry from '../src';
Expand All @@ -12,7 +13,6 @@ import type {
Request,
Response,
} from '../src/gcpfunction/general';

/**
* Why @ts-ignore some Sentry.X calls
*
Expand Down Expand Up @@ -198,7 +198,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -317,7 +317,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -382,7 +382,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -440,7 +440,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -469,7 +469,33 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
});
});

test('marks the captured error as unhandled', async () => {
expect.assertions(4);

const error = new Error('wat');
const handler: EventFunctionWithCallback = (_data, _context, _cb) => {
throw error;
};
const wrappedHandler = wrapEventFunction(handler);
await expect(handleEvent(wrappedHandler)).rejects.toThrowError(error);

expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));

// @ts-ignore just mocking around...
const scopeFunction = SentryNode.captureException.mock.calls[0][1];
const event: Event = { exception: { values: [{}] } };
let evtProcessor: ((e: Event) => Event) | undefined = undefined;
scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) });

expect(evtProcessor).toBeInstanceOf(Function);
// @ts-ignore just mocking around...
expect(evtProcessor(event).exception.values[0].mechanism).toEqual({
handled: false,
type: 'generic',
});
});

Expand Down Expand Up @@ -537,7 +563,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -595,7 +621,7 @@ describe('GCPFunction', () => {
expect(SentryNode.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeTransaction.finish).toBeCalled();
expect(SentryNode.flush).toBeCalled();
Expand Down Expand Up @@ -625,7 +651,7 @@ describe('GCPFunction', () => {
// @ts-ignore see "Why @ts-ignore" note
expect(SentryNode.fakeScope.setSpan).toBeCalledWith(fakeTransaction);

expect(SentryNode.captureException).toBeCalledWith(error);
expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function));
});
});

Expand Down

0 comments on commit 86b3c69

Please sign in to comment.