Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors-reporting: Unhandled rejections are reported #2360

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 comment was marked as spam.

This comment was marked as spam.

'. 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;

This comment was marked as spam.

This comment was marked as spam.


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);
});
});
});
});
});