diff --git a/packages/error-reporting/README.md b/packages/error-reporting/README.md index 54f56ab2b7c..6184b2e10df 100644 --- a/packages/error-reporting/README.md +++ b/packages/error-reporting/README.md @@ -44,6 +44,20 @@ var errors = require('@google-cloud/error-reporting')({ errors.report(new Error('Something broke!')); ``` +- **One may even catch and report application-wide uncaught errors:** + - *It is recommended to catch uncaughtExceptions for production-deployed applications* + - To read more about uncaught exception handling in Node.js and what it means for your application [please click here](https://nodejs.org/api/process.html#process_event_uncaughtexception) + +```js +var errors = require('@google-cloud/error-reporting')(); +process.on('uncaughtException', (e) => { + // Write the error to stderr. + console.error(e); + // Report that same error the Stackdriver Error Service + errors.report(e); +}); +``` + 1. **View reported errors:** Open Stackdriver Error Reporting at https://console.cloud.google.com/errors to view the reported errors. diff --git a/packages/error-reporting/src/configuration.js b/packages/error-reporting/src/configuration.js index 36add709104..d6d86f9e0e7 100644 --- a/packages/error-reporting/src/configuration.js +++ b/packages/error-reporting/src/configuration.js @@ -57,19 +57,6 @@ var Configuration = function(givenConfig, logger) { * @defaultvalue Object */ this._logger = logger; - /** - * The _reportUncaughtExceptions property is meant to contain the optional - * runtime configuration property reportUncaughtExceptions. This property will - * default to true if not given false through the runtime configuration - * meaning that the default behavior is to catch uncaught exceptions, report - * them to the Stackdriver Errors API and then exit. If given false uncaught - * exceptions will not be listened for and not be caught or reported. - * @memberof Configuration - * @private - * @type {Boolean} - * @defaultvalue true - */ - this._reportUncaughtExceptions = true; /** * The _shouldReportErrorsToAPI property is meant to denote whether or not * the Stackdriver error reporting library will actually try to report Errors @@ -276,12 +263,6 @@ Configuration.prototype._gatherLocalConfiguration = function() { 'true in the runtime configuration object' ].join(' ')); } - if (isBoolean(this._givenConfiguration.reportUncaughtExceptions)) { - this._reportUncaughtExceptions = this._givenConfiguration - .reportUncaughtExceptions; - } else if (has(this._givenConfiguration, 'reportUncaughtExceptions')) { - throw new Error('config.reportUncaughtExceptions must be a boolean'); - } if (isString(this._givenConfiguration.key)) { this._key = this._givenConfiguration.key; } else if (has(this._givenConfiguration, 'key')) { @@ -357,16 +338,6 @@ Configuration.prototype._checkLocalProjectId = function(cb) { } return this._projectId; }; -/** - * Returns the _reportUncaughtExceptions property on the instance. - * @memberof Configuration - * @public - * @function getReportUncaughtExceptions - * @returns {Boolean} - returns the _reportUncaughtExceptions property - */ -Configuration.prototype.getReportUncaughtExceptions = function() { - return this._reportUncaughtExceptions; -}; /** * Returns the _shouldReportErrorsToAPI property on the instance. * @memberof Configuration diff --git a/packages/error-reporting/src/index.js b/packages/error-reporting/src/index.js index 8a134e8bd29..3116dd19694 100644 --- a/packages/error-reporting/src/index.js +++ b/packages/error-reporting/src/index.js @@ -25,7 +25,6 @@ var manual = require('./interfaces/manual.js'); var express = require('./interfaces/express.js'); var restify = require('./interfaces/restify'); var messageBuilder = require('./interfaces/message-builder.js'); -var uncaughtException = require('./interfaces/uncaught.js'); var createLogger = require('./logger.js'); /** @@ -84,9 +83,6 @@ function Errors(initConfiguration) { var config = new Configuration(initConfiguration, logger); var client = new AuthClient(config, logger); - // Setup the uncaught exception handler - uncaughtException(client, config); - // Build the application interfaces for use by the hosting application /** * @example diff --git a/packages/error-reporting/src/interfaces/uncaught.js b/packages/error-reporting/src/interfaces/uncaught.js deleted file mode 100644 index e426e5d25e0..00000000000 --- a/packages/error-reporting/src/interfaces/uncaught.js +++ /dev/null @@ -1,67 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var errorHandlerRouter = require('../error-router.js'); -var ErrorMessage = require('../classes/error-message.js'); - -/** - * Exits the process with exit code `1` which indicates that an unhandled error - * occurred. !! Invocation of this function will terminate the process !! - * @function handleProcessExit - * @return {Undefined} - does not return a value - */ -function handleProcessExit() { process.exit(1); } - -/** - * If the configuraiton allows, install an uncaught exception handler that will - * report the uncaught error to the API and then terminate the process. - * @function handlerSetup - * @param {AuthClient} client - the API client for communication with the - * Stackdriver Error API - * @param {Configuration} config - the init configuration - * @returns {Null|process} - Returns null if the config demands ignoring - * uncaught - * exceptions, otherwise return the process instance - */ -function handlerSetup(client, config) { - /** - * The actual exception handler creates a new instance of `ErrorMessage`, - * extracts infomation from the propagated `Error` and marshals it into the - * `ErrorMessage` instance, attempts to send this `ErrorMessage` instance to - * the Stackdriver Error Reporting API. Subsequently the process is - * terminated. - * @function uncaughtExceptionHandler - * @listens module:process~event:uncaughtException - * @param {Error} err - The error that has been uncaught to this point - * @returns {Undefined} - does not return a value - */ - function uncaughtExceptionHandler(err) { - var em = new ErrorMessage(); - errorHandlerRouter(err, em); - client.sendError(em, handleProcessExit); - setTimeout(handleProcessExit, 2000); - } - - if (!config.getReportUncaughtExceptions() || config.lacksCredentials()) { - // Do not attach a listener to the process - return null; - } - - return process.on('uncaughtException', uncaughtExceptionHandler); -} - -module.exports = handlerSetup; diff --git a/packages/error-reporting/test/fixtures/uncaughtExitBehaviour.js b/packages/error-reporting/test/fixtures/uncaughtExitBehaviour.js deleted file mode 100644 index 7f4b8728ad0..00000000000 --- a/packages/error-reporting/test/fixtures/uncaughtExitBehaviour.js +++ /dev/null @@ -1,93 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var uncaughtSetup = require('../../src/interfaces/uncaught.js'); -var nock = require('nock'); -var createLogger = require('../../src/logger.js'); -var isString = require('is').string; -var Configuration = require('../fixtures/configuration.js'); -var RequestHandler = require('../../src/google-apis/auth-client.js'); -var originalHandlers = process.listeners('uncaughtException'); -var UNCAUGHT = 'uncaughtException'; -var client; - -function reattachOriginalListeners() { - for (var i = 0; i < originalHandlers.length; i++) { - process.on(UNCAUGHT, originalHandlers[i]); - } -} -var env = { - NODE_ENV: process.env.NODE_ENV -}; -function setEnv() { - process.env.NODE_ENV = 'production'; -} -function restoreEnv() { - process.env.NODE_ENV = env.NODE_ENV; -} - -describe('Uncaught Exception exit behaviour', function() { - before(function() { - process.removeAllListeners(UNCAUGHT); - if (!isString(process.env.GCLOUD_PROJECT)) { - // The gcloud project id (GCLOUD_PROJECT) was not set as an env variable - this.skip(); - process.exit(1); - } else if (!isString(process.env.GOOGLE_APPLICATION_CREDENTIALS)) { - // The app credentials (GOOGLE_APPLICATION_CREDENTIALS) - // was not set as an env variable - this.skip(); - process.exit(1); - } - setEnv(); - }); - after(function() { - nock.cleanAll(); - nock.enableNetConnect(); - reattachOriginalListeners(); - restoreEnv(); - }); - it('Should attempt to report the uncaught exception', function(done) { - var id = 'xyz'; - nock( - 'http://metadata.google.internal/computeMetadata/v1/project' - ).get('/project-id').times(1).reply(200, id); - nock('https://accounts.google.com:443/o/oauth2') - .post('/token').query(function() {return true;}).reply(200, { - refresh_token: 'hello', - access_token: 'goodbye', - expiry_date: new Date(9999, 1, 1) - }); - this.timeout(2000); - nock( - 'https://clouderrorreporting.googleapis.com/v1beta1/projects/' + id - ).post('/events:report').once().reply(200, function() { - done(); - return {success: true}; - }); - var cfg = new Configuration( - {reportUncaughtExceptions: true, projectId: 'xyz'}); - cfg.lacksCredentials = function() { - return false; - }; - client = new RequestHandler(cfg, createLogger({logLevel: 4})); - uncaughtSetup(client, cfg); - setTimeout(function() { - throw new Error('This error was supposed to be thrown'); - }, 10); - }); -}); diff --git a/packages/error-reporting/test/unit/testConfiguration.js b/packages/error-reporting/test/unit/testConfiguration.js index 13529cfca04..44f96061c81 100644 --- a/packages/error-reporting/test/unit/testConfiguration.js +++ b/packages/error-reporting/test/unit/testConfiguration.js @@ -29,7 +29,6 @@ var nock = require('nock'); var METADATA_URL = 'http://metadata.google.internal/computeMetadata/v1/project'; -process.removeAllListeners('uncaughtException'); var env = { NODE_ENV: process.env.NODE_ENV, GCLOUD_PROJECT: process.env.GCLOUD_PROJECT, @@ -84,12 +83,6 @@ describe('Configuration class', function() { it('Should have a property reflecting the config argument', function() { assert.deepEqual(c._givenConfiguration, stubConfig); }); - it('Should reportUncaughtExceptions', function() { - assert.strictEqual(c.getReportUncaughtExceptions(), true); - }); - it('Should not reportUncaughtExceptions', function() { - assert.strictEqual(c.getShouldReportErrorsToAPI(), false); - }); it('Should not have a project id', function() { assert.strictEqual(c._projectId, null); }); @@ -125,11 +118,6 @@ describe('Configuration class', function() { }); }); describe('exception behaviour', function() { - it('Should throw', function() { - assert.throws(function() { - new Configuration({reportUncaughtExceptions: 1}, logger); - }); - }); it('Should throw if invalid type for key', function() { assert.throws(function() { new Configuration({key: null}, logger); @@ -294,21 +282,6 @@ describe('Configuration class', function() { assert.strictEqual(c.getKey(), key); }); }); - describe('reportUncaughtExceptions', function() { - var c; - var projectId = '123-xyz'; - var reportUncaughtExceptions = false; - before(function() { - c = new Configuration({ - projectId: projectId, - reportUncaughtExceptions: reportUncaughtExceptions - }); - }); - it('Should assign', function() { - assert.strictEqual(c.getReportUncaughtExceptions(), - reportUncaughtExceptions); - }); - }); }); }); }); diff --git a/packages/error-reporting/test/unit/testUncaught.js b/packages/error-reporting/test/unit/testUncaught.js deleted file mode 100644 index 8d9fb503a7e..00000000000 --- a/packages/error-reporting/test/unit/testUncaught.js +++ /dev/null @@ -1,95 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var isString = require('is').string; -var uncaughtSetup = require('../../src/interfaces/uncaught.js'); -var Configuration = require('../fixtures/configuration.js'); -var createLogger = require('../../src/logger.js'); -var originalHandlers = process.listeners('uncaughtException'); -var spawn = require('child_process').spawn; - -function reattachOriginalListeners() { - for (var i = 0; i < originalHandlers.length; i++) { - process.on('uncaughtException', originalHandlers[i]); - } -} - -// Returns a Configuration object with given value for reportUncaughtExceptions, -// and dummy logger -function getConfig(reportUncaughtExceptions) { - var c = new Configuration({ - reportUncaughtExceptions: reportUncaughtExceptions - }, createLogger({logLevel: 4})); - c.lacksCredentials = function() { - return false; - }; - return c; -} - -describe('Uncaught exception handler behvaiour', function() { - var UNCAUGHT = 'uncaughtException'; - describe('Instantiation', function() { - beforeEach(function() {process.removeAllListeners(UNCAUGHT);}); - it('Should throw without a configuration', function() { - assert.throws(uncaughtSetup); - }); - it('Should not throw given a valid configuration', function() { - assert.doesNotThrow(uncaughtSetup.bind(null, {}, getConfig(false))); - assert.doesNotThrow(uncaughtSetup.bind(null, {}, getConfig(true))); - }); - it('Should return the process object after instantiation', function() { - assert.strictEqual(process, uncaughtSetup({}, getConfig(true))); - }); - it('Should not attach a listener to the uncaughtException event if ' + - 'reportUncaughtExceptions is false', function() { - uncaughtSetup({}, getConfig(false)); - assert.strictEqual(process.listeners(UNCAUGHT).length, 0); - }); - it('Should attach a listener to the uncaughtException event if ' + - 'reportUncaughtExceptions is true', function() { - uncaughtSetup({}, getConfig(true)); - assert.strictEqual(process.listeners(UNCAUGHT).length, 1); - }); - }); - describe('Uncaught exception handling shutdown behaviour', function() { - before(function() { - if (!isString(process.env.GOOGLE_APPLICATION_CREDENTIALS) || - !isString(process.env.GCLOUD_PROJECT)) { - return this.skip(); - } - }); - after(function() { - reattachOriginalListeners(); - }); - it('Should terminate before 2500ms', function(done) { - var TERMINATE_MSG = 'Should terminate before 2500ms'; - this.timeout(3500); - var isolate = spawn('./node_modules/mocha/bin/mocha', - ['../../test/fixtures/uncaughtExitBehaviour.js'], {env: process.env}); - isolate.on('close', function() { - done(); - }); - isolate.on('error', function() { - console.log('Test isolate encountered error:', '\n', arguments); - assert(false, TERMINATE_MSG); - done(); - }); - }); - }); -});