diff --git a/MIGRATION.md b/MIGRATION.md index 01addf214ef4..6f8db51f1c9f 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -340,6 +340,7 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p - Remove `SDK_NAME` export from `@sentry/browser`, `@sentry/node`, `@sentry/tracing` and `@sentry/vue` packages. - Removed `eventStatusFromHttpCode` to save on bundle size. - Replace `BrowserTracing` `maxTransactionDuration` option with `finalTimeout` option +- Removed `ignoreSentryErrors` option from AWS lambda SDK. Errors originating from the SDK will now *always* be caught internally. ## Sentry Angular SDK Changes diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 302607e25142..92802bf808cd 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -11,7 +11,7 @@ import { } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; -import { isString, logger, SentryError } from '@sentry/utils'; +import { isString, logger } from '@sentry/utils'; // 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 { Context, Handler } from 'aws-lambda'; @@ -54,7 +54,6 @@ export interface WrapperOptions { * @default false */ captureAllSettledReasons: boolean; - ignoreSentryErrors: boolean; } export const defaultIntegrations: Integration[] = [...Sentry.defaultIntegrations, new AWSServices({ optional: true })]; @@ -226,7 +225,6 @@ export function wrapHandler( captureTimeoutWarning: true, timeoutWarningLimit: 500, captureAllSettledReasons: false, - ignoreSentryErrors: true, ...wrapOptions, }; let timeoutWarningTimer: NodeJS.Timeout; @@ -318,11 +316,7 @@ export function wrapHandler( transaction.finish(); hub.popScope(); await flush(options.flushTimeout).catch(e => { - if (options.ignoreSentryErrors && e instanceof SentryError) { - IS_DEBUG_BUILD && logger.error(e); - return; - } - throw e; + IS_DEBUG_BUILD && logger.error(e); }); } return rv; diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts index f4c79c410d9c..ae993128139f 100644 --- a/packages/serverless/src/gcpfunction/cloud_events.ts +++ b/packages/serverless/src/gcpfunction/cloud_events.ts @@ -61,11 +61,11 @@ function _wrapCloudEventFunction( transaction.finish(); void flush(options.flushTimeout) - .then(() => { - callback(...args); - }) .then(null, e => { IS_DEBUG_BUILD && logger.error(e); + }) + .then(() => { + callback(...args); }); }); diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts index e47efc1571eb..4b29a26270e4 100644 --- a/packages/serverless/src/gcpfunction/events.ts +++ b/packages/serverless/src/gcpfunction/events.ts @@ -56,11 +56,11 @@ function _wrapEventFunction( transaction.finish(); void flush(options.flushTimeout) - .then(() => { - callback(...args); - }) .then(null, e => { IS_DEBUG_BUILD && logger.error(e); + }) + .then(() => { + callback(...args); }); }); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 2e274bad3491..a1eec1e48736 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -91,11 +91,11 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { - _end.call(this, chunk, encoding, cb); - }) .then(null, e => { IS_DEBUG_BUILD && logger.error(e); + }) + .then(() => { + _end.call(this, chunk, encoding, cb); }); }; diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index ab0c38d2f08a..1e49d3d8326f 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -1,4 +1,3 @@ -import { SentryError } from '@sentry/utils'; // 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 { Callback, Handler } from 'aws-lambda'; @@ -178,44 +177,6 @@ describe('AWSLambda', () => { expect(Sentry.captureException).toHaveBeenNthCalledWith(2, error2); expect(Sentry.captureException).toBeCalledTimes(2); }); - - test('ignoreSentryErrors - with successful handler', async () => { - const sentryError = new SentryError('HTTP Error (429)'); - jest.spyOn(Sentry, 'flush').mockRejectedValueOnce(sentryError); - - const handledError = new Error('handled error, and we want to monitor it'); - const expectedSuccessStatus = { status: 'success', reason: 'we handled error, so success' }; - const handler = () => { - Sentry.captureException(handledError); - return Promise.resolve(expectedSuccessStatus); - }; - const wrappedHandlerWithoutIgnoringSentryErrors = wrapHandler(handler, { ignoreSentryErrors: false }); - const wrappedHandlerWithIgnoringSentryErrors = wrapHandler(handler, { ignoreSentryErrors: true }); - - await expect(wrappedHandlerWithoutIgnoringSentryErrors(fakeEvent, fakeContext, fakeCallback)).rejects.toThrow( - sentryError, - ); - await expect(wrappedHandlerWithIgnoringSentryErrors(fakeEvent, fakeContext, fakeCallback)).resolves.toBe( - expectedSuccessStatus, - ); - }); - - test('ignoreSentryErrors - with failed handler', async () => { - const sentryError = new SentryError('HTTP Error (429)'); - jest.spyOn(Sentry, 'flush').mockRejectedValueOnce(sentryError); - - const criticalUnhandledError = new Error('critical unhandled error '); - const handler = () => Promise.reject(criticalUnhandledError); - const wrappedHandlerWithoutIgnoringSentryErrors = wrapHandler(handler, { ignoreSentryErrors: false }); - const wrappedHandlerWithIgnoringSentryErrors = wrapHandler(handler, { ignoreSentryErrors: true }); - - await expect(wrappedHandlerWithoutIgnoringSentryErrors(fakeEvent, fakeContext, fakeCallback)).rejects.toThrow( - sentryError, - ); - await expect(wrappedHandlerWithIgnoringSentryErrors(fakeEvent, fakeContext, fakeCallback)).rejects.toThrow( - criticalUnhandledError, - ); - }); }); describe('wrapHandler() on sync handler', () => { @@ -345,6 +306,21 @@ describe('AWSLambda', () => { expect(Sentry.flush).toBeCalled(); } }); + + test('should not throw when flush rejects', async () => { + const handler: Handler = async () => { + // Friendly handler with no errors :) + return 'some string'; + }; + + const wrappedHandler = wrapHandler(handler); + + jest.spyOn(Sentry, 'flush').mockImplementationOnce(async () => { + throw new Error(); + }); + + await expect(wrappedHandler(fakeEvent, fakeContext, fakeCallback)).resolves.toBe('some string'); + }); }); describe('wrapHandler() on async handler with a callback method (aka incorrect usage)', () => { diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 0a77742a33c6..5072de8b17b7 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -148,6 +148,34 @@ describe('GCPFunction', () => { expect(Sentry.fakeTransaction.finish).toBeCalled(); expect(Sentry.flush).toBeCalled(); }); + + test('should not throw when flush rejects', async () => { + expect.assertions(2); + + const handler: HttpFunction = async (_req, res) => { + res.statusCode = 200; + res.end(); + }; + + const wrappedHandler = wrapHttpFunction(handler); + + const request = { + method: 'POST', + url: '/path?q=query', + headers: { host: 'hostname', 'content-type': 'application/json' }, + body: { foo: 'bar' }, + } as Request; + + const mockEnd = jest.fn(); + const response = { end: mockEnd } as unknown as Response; + + jest.spyOn(Sentry, 'flush').mockImplementationOnce(async () => { + throw new Error(); + }); + + await expect(wrappedHandler(request, response)).resolves.toBeUndefined(); + expect(mockEnd).toHaveBeenCalledTimes(1); + }); }); test('wrapHttpFunction request data', async () => {