From 2e1fe81a2809583f071e336ac05b75254561ad62 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Thu, 20 Apr 2023 11:04:24 +0100 Subject: [PATCH] fix: exit server after reconfiguring According to [the docs][1], `exitOnUncaughtException` should: > Exit the process after handling an uncaught exception. Requires > `captureUncaught` to also be set. So with this config: ```js var rollbar = new Rollbar({ captureUncaught: true, exitOnUncaughtException: true, // ... }); ``` ...an uncaught exception should exit the process. This works correctly, but *only* if `rollbar.configure()` is *never* called again, because: 1. `exitOnUncaughtException` [was deleted][2] from our options 2. Calling `configure()` calls [`setupUnhandledCapture()`][3]... 3. ...which calls [`handleUncaughtExceptions()`][4]... 4. ...which now [tries to access][5] `options.exitOnUncaughtException` 5. But this option was deleted in Step 1, so when we [replace][6] our `uncaughtException` handler, we now have `exitOnUncaught = false` This change: - no longer deletes `exitOnUncaughtException` from options (it's unclear why this is the only option that is deleted) - directly accesses `options.exitOnUncaughtException` in the handler, just like `options.captureUncaught` is accessed directly [1]: https://docs.rollbar.com/docs/rollbarjs-configuration-reference#context-1 [2]: https://github.com/rollbar/rollbar.js/blob/e964ecbdb16a4d2b8b5feaa8b70a2ec327648ed0/src/server/rollbar.js#L587 [3]: https://github.com/rollbar/rollbar.js/blob/e964ecbdb16a4d2b8b5feaa8b70a2ec327648ed0/src/server/rollbar.js#L118 [4]: https://github.com/rollbar/rollbar.js/blob/e964ecbdb16a4d2b8b5feaa8b70a2ec327648ed0/src/server/rollbar.js#L578 [5]: https://github.com/rollbar/rollbar.js/blob/e964ecbdb16a4d2b8b5feaa8b70a2ec327648ed0/src/server/rollbar.js#L586 [6]: https://github.com/rollbar/rollbar.js/blob/e964ecbdb16a4d2b8b5feaa8b70a2ec327648ed0/src/server/rollbar.js#L589 --- src/server/rollbar.js | 5 +---- test/server.rollbar.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/server/rollbar.js b/src/server/rollbar.js index 0ce4d0570..e99b59b2e 100644 --- a/src/server/rollbar.js +++ b/src/server/rollbar.js @@ -583,9 +583,6 @@ Rollbar.prototype.setupUnhandledCapture = function () { }; Rollbar.prototype.handleUncaughtExceptions = function () { - var exitOnUncaught = !!this.options.exitOnUncaughtException; - delete this.options.exitOnUncaughtException; - addOrReplaceRollbarHandler('uncaughtException', function (err) { if (!this.options.captureUncaught && !this.options.handleUncaughtExceptions) { return; @@ -597,7 +594,7 @@ Rollbar.prototype.handleUncaughtExceptions = function () { logger.error(err); } }); - if (exitOnUncaught) { + if (this.options.exitOnUncaughtException) { setImmediate(function () { this.wait(function () { process.exit(1); diff --git a/test/server.rollbar.test.js b/test/server.rollbar.test.js index 21683be03..0e261fa56 100644 --- a/test/server.rollbar.test.js +++ b/test/server.rollbar.test.js @@ -550,6 +550,38 @@ vows.describe('rollbar') assert.equal(logStub.getCall(0).args[0].err.message, 'node error'); } notifier.log.restore(); + }, + 'exitOnUncaughtException': { + topic: function () { + var rollbar = new Rollbar({ + accessToken: 'abc123', + captureUncaught: true, + exitOnUncaughtException: true + }); + rollbar.exitStub = sinon.stub(process, 'exit'); + + nodeThrow(rollbar, this.callback); + }, + 'should exit': function (r) { + var calls = r.exitStub.getCalls(); + r.exitStub.restore(); + assert.equal(calls.length, 1); + }, + 'unrelated option reconfigured': { + topic: function(r) { + r.configure({ + environment: 'new-env' + }); + r.exitStub = sinon.stub(process, 'exit'); + + nodeThrow(r, this.callback); + }, + 'should exit': function(r) { + var calls = r.exitStub.getCalls(); + r.exitStub.restore(); + assert.equal(calls.length, 1); + } + } } } }