From b261a35109ef798136bcdc60d32e5f4a42d68d8e Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Tue, 19 Mar 2019 10:02:33 +0100 Subject: [PATCH] fix: Make OnUncaughtException consider global options Fixes #1925 --- .../src/integrations/onuncaughtexception.ts | 136 +++++++++--------- .../src/integrations/onunhandledrejection.ts | 8 +- 2 files changed, 74 insertions(+), 70 deletions(-) diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index b821ab77e419..281913c545af 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -2,6 +2,7 @@ import { getCurrentHub, Scope } from '@sentry/core'; import { Integration, Severity } from '@sentry/types'; import { logger } from '@sentry/utils/logger'; +import { NodeOptions } from '../backend'; import { defaultOnFatalError } from '../handlers'; /** Global Promise Rejection handler */ @@ -18,10 +19,8 @@ export class OnUncaughtException implements Integration { /** * @inheritDoc */ - public readonly handler: (error: Error) => void = makeErrorHandler( - // tslint:disable-next-line - this._options.onFatalError, - ); + public readonly handler: (error: Error) => void = this._makeErrorHandler(); + /** * @inheritDoc */ @@ -41,74 +40,83 @@ export class OnUncaughtException implements Integration { public setupOnce(): void { global.process.on('uncaughtException', this.handler.bind(this)); } -} -/** - * @hidden - */ -export function makeErrorHandler( - onFatalError: (firstError: Error, secondError?: Error) => void = defaultOnFatalError, -): (error: Error) => void { - const timeout = 2000; - let caughtFirstError: boolean = false; - let caughtSecondError: boolean = false; - let calledFatalError: boolean = false; - let firstError: Error; + /** + * @hidden + */ + private _makeErrorHandler(): (error: Error) => void { + const timeout = 2000; + let caughtFirstError: boolean = false; + let caughtSecondError: boolean = false; + let calledFatalError: boolean = false; + let firstError: Error; + + return (error: Error): void => { + type onFatalErrorHandlerType = (firstError: Error, secondError?: Error) => void; + + let onFatalError: onFatalErrorHandlerType = defaultOnFatalError; + const client = getCurrentHub().getClient(); + + if (this._options.onFatalError) { + onFatalError = this._options.onFatalError; + } else if (client && (client.getOptions() as NodeOptions).onFatalError) { + onFatalError = (client.getOptions() as NodeOptions).onFatalError as onFatalErrorHandlerType; + } - return (error: Error): void => { - if (!caughtFirstError) { - const hub = getCurrentHub(); + if (!caughtFirstError) { + const hub = getCurrentHub(); - // this is the first uncaught error and the ultimate reason for shutting down - // we want to do absolutely everything possible to ensure it gets captured - // also we want to make sure we don't go recursion crazy if more errors happen after this one - firstError = error; - caughtFirstError = true; + // this is the first uncaught error and the ultimate reason for shutting down + // we want to do absolutely everything possible to ensure it gets captured + // also we want to make sure we don't go recursion crazy if more errors happen after this one + firstError = error; + caughtFirstError = true; - if (hub.getIntegration(OnUncaughtException)) { - hub.withScope((scope: Scope) => { - scope.setLevel(Severity.Fatal); - hub.captureException(error, { originalException: error }); + if (hub.getIntegration(OnUncaughtException)) { + hub.withScope((scope: Scope) => { + scope.setLevel(Severity.Fatal); + hub.captureException(error, { originalException: error }); + if (!calledFatalError) { + calledFatalError = true; + onFatalError(error); + } + }); + } else { if (!calledFatalError) { calledFatalError = true; onFatalError(error); } - }); - } else { - if (!calledFatalError) { - calledFatalError = true; - onFatalError(error); } + } else if (calledFatalError) { + // we hit an error *after* calling onFatalError - pretty boned at this point, just shut it down + logger.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown'); + defaultOnFatalError(error); + } else if (!caughtSecondError) { + // two cases for how we can hit this branch: + // - capturing of first error blew up and we just caught the exception from that + // - quit trying to capture, proceed with shutdown + // - a second independent error happened while waiting for first error to capture + // - want to avoid causing premature shutdown before first error capture finishes + // it's hard to immediately tell case 1 from case 2 without doing some fancy/questionable domain stuff + // so let's instead just delay a bit before we proceed with our action here + // in case 1, we just wait a bit unnecessarily but ultimately do the same thing + // in case 2, the delay hopefully made us wait long enough for the capture to finish + // two potential nonideal outcomes: + // nonideal case 1: capturing fails fast, we sit around for a few seconds unnecessarily before proceeding correctly by calling onFatalError + // nonideal case 2: case 2 happens, 1st error is captured but slowly, timeout completes before capture and we treat second error as the sendErr of (nonexistent) failure from trying to capture first error + // note that after hitting this branch, we might catch more errors where (caughtSecondError && !calledFatalError) + // we ignore them - they don't matter to us, we're just waiting for the second error timeout to finish + caughtSecondError = true; + setTimeout(() => { + if (!calledFatalError) { + // it was probably case 1, let's treat err as the sendErr and call onFatalError + calledFatalError = true; + onFatalError(firstError, error); + } else { + // it was probably case 2, our first error finished capturing while we waited, cool, do nothing + } + }, timeout); // capturing could take at least sendTimeout to fail, plus an arbitrary second for how long it takes to collect surrounding source etc } - } else if (calledFatalError) { - // we hit an error *after* calling onFatalError - pretty boned at this point, just shut it down - logger.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown'); - defaultOnFatalError(error); - } else if (!caughtSecondError) { - // two cases for how we can hit this branch: - // - capturing of first error blew up and we just caught the exception from that - // - quit trying to capture, proceed with shutdown - // - a second independent error happened while waiting for first error to capture - // - want to avoid causing premature shutdown before first error capture finishes - // it's hard to immediately tell case 1 from case 2 without doing some fancy/questionable domain stuff - // so let's instead just delay a bit before we proceed with our action here - // in case 1, we just wait a bit unnecessarily but ultimately do the same thing - // in case 2, the delay hopefully made us wait long enough for the capture to finish - // two potential nonideal outcomes: - // nonideal case 1: capturing fails fast, we sit around for a few seconds unnecessarily before proceeding correctly by calling onFatalError - // nonideal case 2: case 2 happens, 1st error is captured but slowly, timeout completes before capture and we treat second error as the sendErr of (nonexistent) failure from trying to capture first error - // note that after hitting this branch, we might catch more errors where (caughtSecondError && !calledFatalError) - // we ignore them - they don't matter to us, we're just waiting for the second error timeout to finish - caughtSecondError = true; - setTimeout(() => { - if (!calledFatalError) { - // it was probably case 1, let's treat err as the sendErr and call onFatalError - calledFatalError = true; - onFatalError(firstError, error); - } else { - // it was probably case 2, our first error finished capturing while we waited, cool, do nothing - } - }, timeout); // capturing could take at least sendTimeout to fail, plus an arbitrary second for how long it takes to collect surrounding source etc - } - }; + }; + } } diff --git a/packages/node/src/integrations/onunhandledrejection.ts b/packages/node/src/integrations/onunhandledrejection.ts index 5461013a6dcf..4389da64f831 100644 --- a/packages/node/src/integrations/onunhandledrejection.ts +++ b/packages/node/src/integrations/onunhandledrejection.ts @@ -41,14 +41,10 @@ export class OnUnhandledRejection implements Integration { scope.setUser(context.user); } if (context.tags) { - Object.keys(context.tags).forEach(key => { - scope.setTag(key, context.tags[key]); - }); + scope.setTags(context.tags); } if (context.extra) { - Object.keys(context.extra).forEach(key => { - scope.setExtra(key, context.extra[key]); - }); + scope.setExtras(context.extra); } hub.captureException(reason, { originalException: promise });