Skip to content

Commit

Permalink
errors-reporting: Unhandled rejections are reported (#2360)
Browse files Browse the repository at this point in the history
  • Loading branch information
DominicKramer authored and stephenplusplus committed Jun 15, 2017
1 parent 3d98532 commit 06e743e
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 34 deletions.
13 changes: 12 additions & 1 deletion packages/error-reporting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,17 @@ errors.report(new Error('Something broke!'));

Open Stackdriver Error Reporting at https://console.cloud.google.com/errors to view the reported errors.

## Unhandled Rejections

Unhandled Rejections are not reported by default. The reporting of unhandled rejections can be enabled using the `reportUnhandledRejections` configuration option. See the [Configuration](#configuration) section for more details.

If unhandled rejections are set to be reported, then, when an unhandled rejection occurs, a message is printed to standard out indicated that an unhandled rejection had occurred and is being reported, and the value causing the rejection is reported to the error-reporting console.

## Catching and Reporting Application-wide Uncaught Errors

*It is recommended to catch `uncaughtExceptions` for production-deployed applications.*
Uncaught exceptions are not reported by default. *It is recommended to process `uncaughtException`s for production-deployed applications.*

Note that uncaught exceptions are not reported by default because to do so would require adding a listener to the `uncaughtException` event. Adding such a listener without knowledge of other `uncaughtException` listeners can cause interference between the event handlers or prevent the process from terminating cleanly. As such, it is necessary for `uncaughtException`s to be reported manually.

```js
var errors = require('@google-cloud/error-reporting')();
Expand Down Expand Up @@ -156,6 +164,9 @@ var errors = require('@google-cloud/error-reporting')({
// should be reported
// defaults to 2 (warnings)
logLevel: 2,
// determines whether or not unhandled rejections are reported to the
// error-reporting console
reportUnhandledRejections: true,
serviceContext: {
service: 'my-service',
version: 'my-service-version'
Expand Down
24 changes: 24 additions & 0 deletions packages/error-reporting/src/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ var Configuration = function(givenConfig, logger) {
* @type {Object}
*/
this._serviceContext = {service: 'nodejs', version: ''};
/**
* The _reportUnhandledRejections property is meant to specify whether or
* not unhandled rejections should be reported to the error-reporting console.
* @memberof Configuration
* @private
* @type {Boolean}
*/
this._reportUnhandledRejections = false;
/**
* The _givenConfiguration property holds a ConfigurationOptions object
* which, if valid, will be merged against by the values taken from the meta-
Expand Down Expand Up @@ -257,6 +265,12 @@ Configuration.prototype._gatherLocalConfiguration = function() {
} else if (has(this._givenConfiguration, 'credentials')) {
throw new Error('config.credentials must be a valid credentials object');
}
if (isBoolean(this._givenConfiguration.reportUnhandledRejections)) {
this._reportUnhandledRejections =
this._givenConfiguration.reportUnhandledRejections;
} else if (has(this._givenConfiguration, 'reportUnhandledRejections')) {
throw new Error('config.reportUnhandledRejections must be a boolean');
}
};
/**
* The _checkLocalProjectId function is responsible for determing whether the
Expand Down Expand Up @@ -354,4 +368,14 @@ Configuration.prototype.getCredentials = function() {
Configuration.prototype.getServiceContext = function() {
return this._serviceContext;
};
/**
* Returns the _reportUnhandledRejections property on the instance.
* @memberof Configuration
* @public
* @function getReportUnhandledRejections
* @returns {Boolean} - returns the _reportUnhandledRejections property
*/
Configuration.prototype.getReportUnhandledRejections = function() {
return this._reportUnhandledRejections;
};
module.exports = Configuration;
11 changes: 11 additions & 0 deletions packages/error-reporting/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,17 @@ function Errors(initConfiguration) {
* app.use(errors.koa);
*/
this.koa = koa(this._client, this._config);

if (this._config.getReportUnhandledRejections()) {
var that = this;
process.on('unhandledRejection', function(reason) {
console.log('UnhandledPromiseRejectionWarning: ' +
'Unhandled promise rejection: ' + reason +
'. This rejection has been reported to the ' +
'Google Cloud Platform error-reporting console.');
that.report(reason);
});
}
}

module.exports = Errors;
144 changes: 111 additions & 33 deletions packages/error-reporting/system-test/testAuthClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ var forEach = require('lodash.foreach');
var assign = require('lodash.assign');
var pick = require('lodash.pick');
var omitBy = require('lodash.omitby');
var util = require('util');

const ERR_TOKEN = '_@google_STACKDRIVER_INTEGRATION_TEST_ERROR__';
const TIMEOUT = 20000;
const TIMEOUT = 30000;

const envKeys = ['GOOGLE_APPLICATION_CREDENTIALS', 'GCLOUD_PROJECT',
'NODE_ENV'];
Expand Down Expand Up @@ -371,54 +372,91 @@ describe('error-reporting', function() {

var errors;
var transport;
var oldLogger;
var logOutput = '';
before(function() {
errors = require('../src/index.js')({
// This test assumes that only the error-reporting library will be
// adding listeners to the 'unhandledRejection' event. Thus we need to
// make sure that no listeners for that event exist. If this check
// fails, then the reinitialize() method below will need to updated to
// more carefully reinitialize the error-reporting library without
// interfering with existing listeners of the 'unhandledRejection' event.
assert.strictEqual(process.listenerCount('unhandledRejection'), 0);
oldLogger = console.log;
console.log = function() {
var text = util.format.apply(null, arguments);
oldLogger(text);
logOutput += text;
};
reinitialize();
});

function reinitialize(extraConfig) {
process.removeAllListeners('unhandledRejection');
var config = Object.assign({
ignoreEnvironmentCheck: true,
serviceContext: {
service: SERVICE_NAME,
version: SERVICE_VERSION
}
});
}, extraConfig || {});
errors = require('../src/index.js')(config);
transport = new ErrorsApiTransport(errors._config, errors._logger);
});
}

after(function(done) {
transport.deleteAllEvents(function(err) {
assert.ifError(err);
done();
});
console.log = oldLogger;
if (transport) {
transport.deleteAllEvents(function(err) {
assert.ifError(err);
done();
});
}
});

afterEach(function() {
logOutput = '';
});

function verifyAllGroups(messageTest, timeout, cb) {
setTimeout(function() {
transport.getAllGroups(function(err, groups) {
assert.ifError(err);
assert.ok(groups);

var matchedErrors = groups.filter(function(errItem) {
return errItem && errItem.representative &&
messageTest(errItem.representative.message);
});

cb(matchedErrors);
});
}, timeout);
}

function verifyServerResponse(messageTest, timeout, cb) {
verifyAllGroups(messageTest, timeout, function(matchedErrors) {
// The error should have been reported exactly once
assert.strictEqual(matchedErrors.length, 1);
var errItem = matchedErrors[0];
assert.ok(errItem);
assert.equal(errItem.count, 1);
var rep = errItem.representative;
assert.ok(rep);
var context = rep.serviceContext;
assert.ok(context);
assert.strictEqual(context.service, SERVICE_NAME);
assert.strictEqual(context.version, SERVICE_VERSION);
cb();
});
}

function verifyReporting(errOb, messageTest, timeout, cb) {
errors.report(errOb, function(err, response, body) {
assert.ifError(err);
assert(isObject(response));
assert.deepEqual(body, {});

setTimeout(function() {
transport.getAllGroups(function(err, groups) {
assert.ifError(err);
assert.ok(groups);

var matchedErrors = groups.filter(function(errItem) {
return errItem && errItem.representative &&
messageTest(errItem.representative.message);
});

// The error should have been reported exactly once
assert.strictEqual(matchedErrors.length, 1);
var errItem = matchedErrors[0];
assert.ok(errItem);
assert.equal(errItem.count, 1);
var rep = errItem.representative;
assert.ok(rep);
var context = rep.serviceContext;
assert.ok(context);
assert.strictEqual(context.service, SERVICE_NAME);
assert.strictEqual(context.version, SERVICE_VERSION);
cb();
});
}, timeout);
verifyServerResponse(messageTest, timeout, cb);
});
}

Expand Down Expand Up @@ -465,4 +503,44 @@ describe('error-reporting', function() {
}, TIMEOUT, done);
})();
});

it('Should report unhandledRejections if enabled', function(done) {
this.timeout(TIMEOUT * 2);
reinitialize({ reportUnhandledRejections: true });
var rejectValue = buildName('promise-rejection');
Promise.reject(rejectValue);
setImmediate(function() {
var expected = 'UnhandledPromiseRejectionWarning: Unhandled ' +
'promise rejection: ' + rejectValue +
'. This rejection has been reported to the ' +
'Google Cloud Platform error-reporting console.';
assert.notStrictEqual(logOutput.indexOf(expected), -1);
verifyServerResponse(function(message) {
return message.startsWith(rejectValue);
}, TIMEOUT, done);
});
});

it('Should not report unhandledRejections if disabled', function(done) {
this.timeout(TIMEOUT * 2);
reinitialize({ reportUnhandledRejections: false });
var rejectValue = buildName('promise-rejection');
Promise.reject(rejectValue);
setImmediate(function() {
var notExpected = 'UnhandledPromiseRejectionWarning: Unhandled ' +
'promise rejection: ' + rejectValue +
'. This rejection has been reported to the error-reporting console.';
assert.strictEqual(logOutput.indexOf(notExpected), -1);
// Get all groups that that start with the rejection value and hence all
// of the groups corresponding to the above rejection (Since the
// buildName() creates a string unique enough to single out only the
// above rejection.) and verify that there are no such groups reported.
verifyAllGroups(function(message) {
return message.startsWith(rejectValue);
}, TIMEOUT, function(matchedErrors) {
assert.strictEqual(matchedErrors.length, 0);
done();
});
});
});
});
23 changes: 23 additions & 0 deletions packages/error-reporting/test/unit/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ describe('Configuration class', function() {
assert.deepEqual(c.getServiceContext(),
{service: 'node', version: undefined});
});
it('Should specify to not report unhandledRejections', function() {
assert.strictEqual(c.getReportUnhandledRejections(), false);
});
});
describe('with ignoreEnvironmentCheck', function() {
var conf = merge({}, stubConfig, {ignoreEnvironmentCheck: true});
Expand Down Expand Up @@ -134,6 +137,13 @@ describe('Configuration class', function() {
new Configuration({serviceContext: {version: true}}, logger);
});
});
it('Should throw if invalid for reportUnhandledRejections',
function() {
assert.throws(function() {
new Configuration({ reportUnhandledRejections: 'INVALID' },
logger);
});
});
it('Should not throw given an empty object for serviceContext',
function() {
assert.doesNotThrow(function() {
Expand Down Expand Up @@ -278,6 +288,19 @@ describe('Configuration class', function() {
assert.strictEqual(c.getKey(), key);
});
});
describe('reportUnhandledRejections', function() {
var c;
var reportRejections = false;
before(function() {
c = new Configuration({
reportUnhandledRejections: reportRejections
});
});
it('Should assign', function() {
assert.strictEqual(c.getReportUnhandledRejections(),
reportRejections);
});
});
});
});
});

0 comments on commit 06e743e

Please sign in to comment.