Skip to content

Commit 41c0f4e

Browse files
committed
fix: Make OnUncaughtException consider global options
Fixes getsentry#1925
1 parent 98d8260 commit 41c0f4e

File tree

2 files changed

+74
-70
lines changed

2 files changed

+74
-70
lines changed

packages/node/src/integrations/onuncaughtexception.ts

Lines changed: 72 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getCurrentHub, Scope } from '@sentry/core';
22
import { Integration, Severity } from '@sentry/types';
33
import { logger } from '@sentry/utils/logger';
44

5+
import { NodeOptions } from '../backend';
56
import { defaultOnFatalError } from '../handlers';
67

78
/** Global Promise Rejection handler */
@@ -18,10 +19,8 @@ export class OnUncaughtException implements Integration {
1819
/**
1920
* @inheritDoc
2021
*/
21-
public readonly handler: (error: Error) => void = makeErrorHandler(
22-
// tslint:disable-next-line
23-
this._options.onFatalError,
24-
);
22+
public readonly handler: (error: Error) => void = this._makeErrorHandler();
23+
2524
/**
2625
* @inheritDoc
2726
*/
@@ -41,74 +40,83 @@ export class OnUncaughtException implements Integration {
4140
public setupOnce(): void {
4241
global.process.on('uncaughtException', this.handler.bind(this));
4342
}
44-
}
4543

46-
/**
47-
* @hidden
48-
*/
49-
export function makeErrorHandler(
50-
onFatalError: (firstError: Error, secondError?: Error) => void = defaultOnFatalError,
51-
): (error: Error) => void {
52-
const timeout = 2000;
53-
let caughtFirstError: boolean = false;
54-
let caughtSecondError: boolean = false;
55-
let calledFatalError: boolean = false;
56-
let firstError: Error;
44+
/**
45+
* @hidden
46+
*/
47+
private _makeErrorHandler(): (error: Error) => void {
48+
const timeout = 2000;
49+
let caughtFirstError: boolean = false;
50+
let caughtSecondError: boolean = false;
51+
let calledFatalError: boolean = false;
52+
let firstError: Error;
53+
54+
return (error: Error): void => {
55+
type onFatalErrorHandlerType = (firstError: Error, secondError?: Error) => void;
56+
57+
let onFatalError: onFatalErrorHandlerType = defaultOnFatalError;
58+
const client = getCurrentHub().getClient();
59+
60+
if (this._options.onFatalError) {
61+
onFatalError = this._options.onFatalError;
62+
} else if (client && (client.getOptions() as NodeOptions).onFatalError) {
63+
onFatalError = (client.getOptions() as NodeOptions).onFatalError as onFatalErrorHandlerType;
64+
}
5765

58-
return (error: Error): void => {
59-
if (!caughtFirstError) {
60-
const hub = getCurrentHub();
66+
if (!caughtFirstError) {
67+
const hub = getCurrentHub();
6168

62-
// this is the first uncaught error and the ultimate reason for shutting down
63-
// we want to do absolutely everything possible to ensure it gets captured
64-
// also we want to make sure we don't go recursion crazy if more errors happen after this one
65-
firstError = error;
66-
caughtFirstError = true;
69+
// this is the first uncaught error and the ultimate reason for shutting down
70+
// we want to do absolutely everything possible to ensure it gets captured
71+
// also we want to make sure we don't go recursion crazy if more errors happen after this one
72+
firstError = error;
73+
caughtFirstError = true;
6774

68-
if (hub.getIntegration(OnUncaughtException)) {
69-
hub.withScope((scope: Scope) => {
70-
scope.setLevel(Severity.Fatal);
71-
hub.captureException(error, { originalException: error });
75+
if (hub.getIntegration(OnUncaughtException)) {
76+
hub.withScope((scope: Scope) => {
77+
scope.setLevel(Severity.Fatal);
78+
hub.captureException(error, { originalException: error });
79+
if (!calledFatalError) {
80+
calledFatalError = true;
81+
onFatalError(error);
82+
}
83+
});
84+
} else {
7285
if (!calledFatalError) {
7386
calledFatalError = true;
7487
onFatalError(error);
7588
}
76-
});
77-
} else {
78-
if (!calledFatalError) {
79-
calledFatalError = true;
80-
onFatalError(error);
8189
}
90+
} else if (calledFatalError) {
91+
// we hit an error *after* calling onFatalError - pretty boned at this point, just shut it down
92+
logger.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown');
93+
defaultOnFatalError(error);
94+
} else if (!caughtSecondError) {
95+
// two cases for how we can hit this branch:
96+
// - capturing of first error blew up and we just caught the exception from that
97+
// - quit trying to capture, proceed with shutdown
98+
// - a second independent error happened while waiting for first error to capture
99+
// - want to avoid causing premature shutdown before first error capture finishes
100+
// it's hard to immediately tell case 1 from case 2 without doing some fancy/questionable domain stuff
101+
// so let's instead just delay a bit before we proceed with our action here
102+
// in case 1, we just wait a bit unnecessarily but ultimately do the same thing
103+
// in case 2, the delay hopefully made us wait long enough for the capture to finish
104+
// two potential nonideal outcomes:
105+
// nonideal case 1: capturing fails fast, we sit around for a few seconds unnecessarily before proceeding correctly by calling onFatalError
106+
// 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
107+
// note that after hitting this branch, we might catch more errors where (caughtSecondError && !calledFatalError)
108+
// we ignore them - they don't matter to us, we're just waiting for the second error timeout to finish
109+
caughtSecondError = true;
110+
setTimeout(() => {
111+
if (!calledFatalError) {
112+
// it was probably case 1, let's treat err as the sendErr and call onFatalError
113+
calledFatalError = true;
114+
onFatalError(firstError, error);
115+
} else {
116+
// it was probably case 2, our first error finished capturing while we waited, cool, do nothing
117+
}
118+
}, timeout); // capturing could take at least sendTimeout to fail, plus an arbitrary second for how long it takes to collect surrounding source etc
82119
}
83-
} else if (calledFatalError) {
84-
// we hit an error *after* calling onFatalError - pretty boned at this point, just shut it down
85-
logger.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown');
86-
defaultOnFatalError(error);
87-
} else if (!caughtSecondError) {
88-
// two cases for how we can hit this branch:
89-
// - capturing of first error blew up and we just caught the exception from that
90-
// - quit trying to capture, proceed with shutdown
91-
// - a second independent error happened while waiting for first error to capture
92-
// - want to avoid causing premature shutdown before first error capture finishes
93-
// it's hard to immediately tell case 1 from case 2 without doing some fancy/questionable domain stuff
94-
// so let's instead just delay a bit before we proceed with our action here
95-
// in case 1, we just wait a bit unnecessarily but ultimately do the same thing
96-
// in case 2, the delay hopefully made us wait long enough for the capture to finish
97-
// two potential nonideal outcomes:
98-
// nonideal case 1: capturing fails fast, we sit around for a few seconds unnecessarily before proceeding correctly by calling onFatalError
99-
// 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
100-
// note that after hitting this branch, we might catch more errors where (caughtSecondError && !calledFatalError)
101-
// we ignore them - they don't matter to us, we're just waiting for the second error timeout to finish
102-
caughtSecondError = true;
103-
setTimeout(() => {
104-
if (!calledFatalError) {
105-
// it was probably case 1, let's treat err as the sendErr and call onFatalError
106-
calledFatalError = true;
107-
onFatalError(firstError, error);
108-
} else {
109-
// it was probably case 2, our first error finished capturing while we waited, cool, do nothing
110-
}
111-
}, timeout); // capturing could take at least sendTimeout to fail, plus an arbitrary second for how long it takes to collect surrounding source etc
112-
}
113-
};
120+
};
121+
}
114122
}

packages/node/src/integrations/onunhandledrejection.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,10 @@ export class OnUnhandledRejection implements Integration {
4141
scope.setUser(context.user);
4242
}
4343
if (context.tags) {
44-
Object.keys(context.tags).forEach(key => {
45-
scope.setTag(key, context.tags[key]);
46-
});
44+
scope.setTags(context.tags);
4745
}
4846
if (context.extra) {
49-
Object.keys(context.extra).forEach(key => {
50-
scope.setExtra(key, context.extra[key]);
51-
});
47+
scope.setExtras(context.extra);
5248
}
5349

5450
hub.captureException(reason, { originalException: promise });

0 commit comments

Comments
 (0)