Skip to content

Commit 89156a4

Browse files
J4YF7OJ4YM
and
J4YM
authored
feat(awslambda) : Capture errors individually on sqs partial batch failure (#4130)
A lambda can receive up to 1000 messages. (if we use a batchWindow), but it can fail because of 10/1000 messages failed. Tools such as [Middy](https://middy.js.org/) (and the `sqs-partial-batch-failure` plugin) allow managing this case by deleting records from the queue individually, so that we don't have to retry a record that has already worked. This PR suggests adding an option to capture all errors coming from a `Promise.allSettled`. The `wrapHandler` will not fail the lambda even if there are errors, because I assume that it will be the middy’s job. Co-authored-by: J4YM <[email protected]>
1 parent 6b278f1 commit 89156a4

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

packages/serverless/src/awslambda.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/* eslint-disable max-lines */
2+
import * as Sentry from '@sentry/node';
13
import {
24
captureException,
35
captureMessage,
@@ -8,7 +10,6 @@ import {
810
startTransaction,
911
withScope,
1012
} from '@sentry/node';
11-
import * as Sentry from '@sentry/node';
1213
import { extractTraceparentData } from '@sentry/tracing';
1314
import { Integration } from '@sentry/types';
1415
import { isString, logger } from '@sentry/utils';
@@ -46,6 +47,12 @@ export interface WrapperOptions {
4647
callbackWaitsForEmptyEventLoop: boolean;
4748
captureTimeoutWarning: boolean;
4849
timeoutWarningLimit: number;
50+
/**
51+
* Capture all errors when `Promise.allSettled` is returned by the handler
52+
* The {@link wrapHandler} will not fail the lambda even if there are errors
53+
* @default false
54+
*/
55+
captureAllSettledReasons: boolean;
4956
}
5057

5158
export const defaultIntegrations: Integration[] = [...Sentry.defaultIntegrations, new AWSServices({ optional: true })];
@@ -86,9 +93,29 @@ function tryRequire<T>(taskRoot: string, subdir: string, mod: string): T {
8693
return require(require.resolve(mod, { paths: [taskRoot, subdir] }));
8794
}
8895

96+
/** */
97+
function isPromiseAllSettledResult<T>(result: T[]): boolean {
98+
return result.every(
99+
v =>
100+
Object.prototype.hasOwnProperty.call(v, 'status') &&
101+
(Object.prototype.hasOwnProperty.call(v, 'value') || Object.prototype.hasOwnProperty.call(v, 'reason')),
102+
);
103+
}
104+
105+
type PromiseSettledResult<T> = { status: 'rejected' | 'fulfilled'; reason?: T };
106+
107+
/** */
108+
function getRejectedReasons<T>(results: PromiseSettledResult<T>[]): T[] {
109+
return results.reduce((rejected: T[], result) => {
110+
if (result.status === 'rejected' && result.reason) rejected.push(result.reason);
111+
return rejected;
112+
}, []);
113+
}
114+
89115
/** */
90116
export function tryPatchHandler(taskRoot: string, handlerPath: string): void {
91117
type HandlerBag = HandlerModule | Handler | null | undefined;
118+
92119
interface HandlerModule {
93120
[key: string]: HandlerBag;
94121
}
@@ -197,6 +224,7 @@ export function wrapHandler<TEvent, TResult>(
197224
callbackWaitsForEmptyEventLoop: false,
198225
captureTimeoutWarning: true,
199226
timeoutWarningLimit: 500,
227+
captureAllSettledReasons: false,
200228
...wrapOptions,
201229
};
202230
let timeoutWarningTimer: NodeJS.Timeout;
@@ -272,6 +300,14 @@ export function wrapHandler<TEvent, TResult>(
272300
// We put the transaction on the scope so users can attach children to it
273301
scope.setSpan(transaction);
274302
rv = await asyncHandler(event, context);
303+
304+
// We manage lambdas that use Promise.allSettled by capturing the errors of failed promises
305+
if (options.captureAllSettledReasons && Array.isArray(rv) && isPromiseAllSettledResult(rv)) {
306+
const reasons = getRejectedReasons(rv);
307+
reasons.forEach(exception => {
308+
captureException(exception);
309+
});
310+
}
275311
} catch (e) {
276312
captureException(e);
277313
if (options.rethrowAfterCapture) {

packages/serverless/test/awslambda.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,29 @@ describe('AWSLambda', () => {
169169
// @ts-ignore see "Why @ts-ignore" note
170170
expect(Sentry.fakeScope.setTag).toBeCalledWith('timeout', '1m40s');
171171
});
172+
173+
test('captureAllSettledReasons disabled (default)', async () => {
174+
const handler = () => Promise.resolve([{ status: 'rejected', reason: new Error() }]);
175+
const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337 });
176+
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
177+
expect(Sentry.captureException).toBeCalledTimes(0);
178+
});
179+
180+
test('captureAllSettledReasons enable', async () => {
181+
const error = new Error();
182+
const error2 = new Error();
183+
const handler = () =>
184+
Promise.resolve([
185+
{ status: 'rejected', reason: error },
186+
{ status: 'fulfilled', value: undefined },
187+
{ status: 'rejected', reason: error2 },
188+
]);
189+
const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337, captureAllSettledReasons: true });
190+
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
191+
expect(Sentry.captureException).toHaveBeenNthCalledWith(1, error);
192+
expect(Sentry.captureException).toHaveBeenNthCalledWith(2, error2);
193+
expect(Sentry.captureException).toBeCalledTimes(2);
194+
});
172195
});
173196

174197
describe('wrapHandler() on sync handler', () => {

0 commit comments

Comments
 (0)