Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Commit

Permalink
Changes to match updated spec
Browse files Browse the repository at this point in the history
  • Loading branch information
@bengourley
bengourley committed Sep 20, 2017
1 parent c042167 commit de9d65f
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 77 deletions.
52 changes: 26 additions & 26 deletions lib/bugsnag.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,10 @@ Bugsnag.configure = function(options) {
unCaughtErrorHandlerAdded = true;
Configuration.logger.info("Configuring uncaughtExceptionHandler");
process.on("uncaughtException", function(err) {
notify(err, {
severity: "error"
}, {
defaultSeverity: true,
notify(err, {}, {
originalSeverity: "error",
unhandled: true,
severityReason: { type: "exception_handler" }
severityReason: { type: "unhandledException" }
}, autoNotifyCallback(err, true));
});
}
Expand All @@ -147,12 +145,10 @@ Bugsnag.configure = function(options) {
unhandledRejectionHandlerAdded = true;
Configuration.logger.info("Configuring unhandledRejectionHandler");
process.on("unhandledRejection", function(err) {
notify(err, {
severity: "error"
}, {
defaultSeverity: true,
notify(err, {}, {
originalSeverity: "error",
unhandled: true,
severityReason: { type: "promise_rejection" }
severityReason: { type: "unhandledPromiseRejection" }
}, autoNotifyCallback(err, true));
});
}
Expand All @@ -168,9 +164,9 @@ Bugsnag.notify = function(error, options, cb) {
options = {};
}
return notify(error, options, {
defaultSeverity: options.severity === undefined,
originalSeverity: "warning",
unhandled: false,
severityReason: undefined
severityReason: { type: 'handledException' }
}, cb);
}

Expand All @@ -179,11 +175,10 @@ Bugsnag.errorHandler = function(err, req, res, next) {
Configuration.logger.info("Handling express error: " + (err.stack || err));
notify(err, {
req: req,
severity: "error"
}, {
defaultSeverity: true,
originalSeverity: "error",
unhandled: true,
severityReason: { type: "middleware_handler", name: "express/connect" }
severityReason: { type: "unhandledErrorMiddleware", attributes: { framework: "Express/Connect" } }
}, autoNotifyCallback(err));
return next(err);
};
Expand All @@ -210,12 +205,11 @@ Bugsnag.createRequestHandler = function() {

Bugsnag.restifyHandler = function(req, res, route, err) {
notify(err, {
req: req,
severity: "error"
req: req
}, {
defaultSeverity: true,
originalSeverity: "error",
unhandled: true,
severityReason: { type: "middleware_handler", name: "restify" }
severityReason: { type: "unhandledErrorMiddleware", attributes: { framework: "Restify" } }
}, autoNotifyCallback(err));
};

Expand All @@ -226,12 +220,11 @@ Bugsnag.koaHandler = function(err, ctx) {
request.protocol = ctx.request.protocol;
request.host = ctx.request.host.split(':', 1)[0];
return notify(err, Object.assign({
req: request,
severity: "error"
req: request
}, ctx.bugsnag), {
defaultSeverity: true,
originalSeverity: "error",
unhandled: true,
severityReason: { type: "middleware_handler", name: "koa" }
severityReason: { type: "unhandledErrorMiddleware", attributes: { framework: "Koa" } }
}, autoNotifyCallback(err));
};

Expand All @@ -252,7 +245,11 @@ Bugsnag.intercept = function(options, cb) {
var err = arguments[0];
var args = Array.prototype.slice.call(arguments, 1);
if (err && (err instanceof Error)) {
return Bugsnag.notify(err, options, autoNotifyCallback(err));
return notify(err, options, {
originalSeverity: "warning",
unhandled: false,
severityReason: { type: "callbackErrorIntercept" }
}, autoNotifyCallback(err));
}
if (cb) {
return cb.apply(null, args);
Expand All @@ -272,9 +269,12 @@ Bugsnag.autoNotify = function(options, cb) {
}
dom = domain.create();
dom._bugsnagOptions = options;
options.severity = "error";
dom.on('error', function(err) {
return Bugsnag.notify(err, options, autoNotifyCallback(err));
return notify(err, options, {
originalSeverity: "error",
unhandled: true,
severityReason: { type: "unhandledException" }
}, autoNotifyCallback(err, true));
});
process.nextTick(function() {
return dom.run(cb);
Expand Down
67 changes: 34 additions & 33 deletions lib/notification.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,9 @@ function Notification(bugsnagErrors, options, handledState) {
event.payloadVersion = Configuration.payloadVersion;
}


event.defaultSeverity = handledState.defaultSeverity;
event.unhandled = handledState.unhandled;
event.severityReason = handledState.severityReason;

if (options.severity && SUPPORTED_SEVERITIES.indexOf(options.severity) >= 0) {
event.severity = options.severity;
} else {
event.severity = "warning";
// If defaultSeverity was set to false but we got here, i.e. the user set
// it to something that wasnt in SUPPORTED_SEVERITIES, that value has now
// been set to the default value – "warning".
// The event now does how have the default severity so we annotate it
// appropriately.
event.defaultSeverity = true;
}
// severity and handledState
if (options.severity) event.severity = options.severity;
this.handledState = handledState;

if (Configuration.metaData && Object.keys(Configuration.metaData).length > 0) {
event.metaData = Utils.cloneObject(Configuration.metaData);
Expand Down Expand Up @@ -123,15 +110,18 @@ function Notification(bugsnagErrors, options, handledState) {

Notification.prototype.deliver = function(cb, originalError) {

// save handledState properties, because the user shouldn't be able to set them
var before = {
severity: this.events[0].severity,
defaultSeverity: this.events[0].defaultSeverity,
unhandled: this.events[0].unhandled,
severityReason: this.events[0].severityReason
// did the user set severity?
var userSeverity = this.events[0].severity;
var userSpecifiedSeverity = userSeverity && userSeverity !== this.handledState.originalSeverity;

// annotate this property so that we can check if the "beforeNotify" callbacks changed it
// can't annotate a string literal ""/'', so create a String() object, woo!
if (userSpecifiedSeverity) {
this.events[0].severity = new String(userSpecifiedSeverity);
this.events[0].severity.__userSpecifiedSeverity = true;
}

// run before notify callbacks
// run "beforeNotify" callbacks
var shouldNotify = true;
Configuration.beforeNotifyCallbacks.forEach(function (callback) {
var ret = callback(this, originalError);
Expand All @@ -140,23 +130,34 @@ Notification.prototype.deliver = function(cb, originalError) {
}
}.bind(this));

// did any of the the callbacks set the severity?
var callbackSeverity = this.events[0].severity;
var callbackSetSeverity = callbackSeverity && !callbackSeverity.__userSpecifiedSeverity && callbackSeverity !== this.handledState.originalSeverity;

if (callbackSetSeverity && SUPPORTED_SEVERITIES.indexOf(callbackSeverity) !== -1) {
// callbacks set a valid severity value
this.events[0].severity = callbackSeverity;
this.events[0].severityReason = { type: "userCallbackSetSeverity" };
} else if (userSpecifiedSeverity && SUPPORTED_SEVERITIES.indexOf(userSeverity) !== -1) {
// user specified a valid severity value
this.events[0].severity = userSeverity;
this.events[0].severityReason = { type: "userSpecifiedSeverity" };
} else {
// user did not specify severity, or specified and invalid value
this.events[0].severity = this.handledState.originalSeverity;
this.events[0].severityReason = this.handledState.severityReason;
}

// finally, add whether the error was unhandled so that callbacks can't fiddle with it
this.events[0].unhandled = this.handledState.unhandled;

if (!shouldNotify) {
if (cb) {
cb(new Error("At least one beforeNotify callback prevented the event from being sent to Bugsnag."));
}
return;
}

// reinstate handledState properties
this.events[0].defaultSeverity = before.defaultSeverity
this.events[0].unhandled = before.unhandled
this.events[0].severityReason = before.severityReason

// but do one last check to see if severity was changed as a result of the callback(s)
if (before.severity !== this.events[0].severity) {
this.events[0].defaultSeverity = false;
}

this._deliver(cb);
};

Expand Down
6 changes: 3 additions & 3 deletions test/express.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ describe("express middleware", function() {
var deliverStub;
deliverStub = null;
beforeEach(function() {
return deliverStub = sinon.stub(Notification.prototype, "deliver");
return deliverStub = sinon.stub(Notification.prototype, "_deliver");
});
afterEach(function() {
return Notification.prototype.deliver.restore();
return Notification.prototype._deliver.restore();
});
return it("should automatically collect request data", function(next) {
var app, port;
Expand Down Expand Up @@ -49,7 +49,7 @@ describe("express middleware", function() {
// check the event got the correct handled/unhandled properties set
assert.equal(event.unhandled, true);
assert.equal(event.severity, "error");
assert.deepEqual(event.severityReason, { type: "middleware_handler", name: "express/connect" });
assert.deepEqual(event.severityReason, { type: "unhandledErrorMiddleware", attributes: { framework: "Express/Connect" } });
return next();
});
});
Expand Down
27 changes: 12 additions & 15 deletions test/notification.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe("Notification", function() {
it("should have the correct notification format", function() {
Bugsnag.notify("This is the message");
deliverStub.firstCall.thisValue.should.be.an("object");
deliverStub.firstCall.thisValue.should.have.keys(["apiKey", "notifier", "events"]);
deliverStub.firstCall.thisValue.should.have.keys(["apiKey", "notifier", "events", "handledState"]);
});

it("should identify the notifier properly", function() {
Expand Down Expand Up @@ -107,7 +107,7 @@ describe("Notification", function() {
deliverStub.firstCall.thisValue.events.length.should.equal(1);
deliverStub.firstCall.thisValue.events[0].should.have.keys(
"releaseStage", "exceptions", "device", "payloadVersion", "severity",
"metaData", "unhandled", "severityReason", "defaultSeverity"
"metaData", "unhandled", "severityReason"
);
});

Expand Down Expand Up @@ -355,19 +355,19 @@ describe("Notification", function() {
it("should not be able to modify handledState", function () {
Bugsnag.onBeforeNotify(function (notification, error) {
notification.events[0].unhandled = true;
notification.events[0].defaultSeverity = false;
notification.events[0].severityReason = { something: "else" };
});
Bugsnag.notify(new Error('breaky'));
deliverStub.firstCall.thisValue.events[0].unhandled.should.equal(false);
deliverStub.firstCall.thisValue.events[0].defaultSeverity.should.equal(true);
deliverStub.firstCall.thisValue.events[0].severityReason.should.eql({ type: 'handledException' });
});

it("should cause defaultSeverity=false if any callback changes severity", function () {
it("should cause severityReason = { type: 'userCallbackSetSeverity' } if any callback changes severity", function () {
Bugsnag.onBeforeNotify(function (notification, error) {
notification.events[0].severity = "error";
});
Bugsnag.notify(new Error('breaky'));
deliverStub.firstCall.thisValue.events[0].defaultSeverity.should.equal(false);
deliverStub.firstCall.thisValue.events[0].severityReason.should.eql({ type: 'userCallbackSetSeverity' });
})
});

Expand Down Expand Up @@ -431,9 +431,8 @@ describe("Notification", function() {
deliverCalled.should.equal(true);
var event = payload.events[0]
event.severity.should.equal("error");
event.defaultSeverity.should.equal(true);
event.unhandled.should.equal(true);
event.severityReason.should.eql({ type: "exception_handler" });
event.severityReason.should.eql({ type: "unhandledException" });
done();
} catch (e) {
done(e);
Expand All @@ -455,9 +454,8 @@ describe("Notification", function() {
deliverCalled.should.equal(true);
var event = payload.events[0]
event.severity.should.equal("error");
event.defaultSeverity.should.equal(true);
event.unhandled.should.equal(true);
event.severityReason.should.eql({ type: "promise_rejection" });
event.severityReason.should.eql({ type: "unhandledPromiseRejection" });
done();
} catch (e) {
done(e);
Expand All @@ -470,15 +468,14 @@ describe("Notification", function() {
it("should not allow the user to set handledState properties", function (done) {
Bugsnag.notify("this is an outrage", {
severity: "info",
defaultSeverity: true,
unhandled: true,
severityReason: { type: "naughty" }
});
var event = deliverStub.firstCall.thisValue.events[0];
event.unhandled.should.equal(false);
should.not.exist(event.severityReason);
event.defaultSeverity.should.equal(false);
event.severity.should.equal("info");
event.severityReason.should.eql({ type: "userSpecifiedSeverity" });
done();
})
})
});
});
});

0 comments on commit de9d65f

Please sign in to comment.