From 4b714bcf53d40dd1fbeb4eccb97467d124981b78 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Fri, 21 Apr 2017 12:51:34 -0700 Subject: [PATCH] Remove checks for valid credentials Instead, the `@google-cloud/common` module will handle credential verification. Without this fix, the `error-reporting` library's checks for valid credentials were incorrect and would think that communication with the API would fail even though that was not the case. As a result, the library would not attempt to communicate with the API even though the communication would succeed. With this change, the error reporting library will always communicate with the API and will leave it to the `@google-cloud/common` module to handle issues with invalid credentials. Fixes #2245 - `error-reporting` Only Supports Authentication Using a Service Account Key. --- packages/error-reporting/src/configuration.js | 44 ------------------- .../error-reporting/src/interfaces/express.js | 4 -- .../error-reporting/src/interfaces/hapi.js | 18 +++----- .../error-reporting/src/interfaces/koa.js | 7 +-- .../error-reporting/src/interfaces/manual.js | 3 -- .../error-reporting/src/interfaces/restify.js | 3 -- 6 files changed, 6 insertions(+), 73 deletions(-) diff --git a/packages/error-reporting/src/configuration.js b/packages/error-reporting/src/configuration.js index a83188b8c45..18892607b55 100644 --- a/packages/error-reporting/src/configuration.js +++ b/packages/error-reporting/src/configuration.js @@ -22,8 +22,6 @@ var isObject = is.object; var isBoolean = is.boolean; var isString = is.string; var isNumber = is.number; -var isEmpty = is.empty; -var isNull = is.null; var version = require('../package.json').version; /** @@ -146,15 +144,6 @@ var Configuration = function(givenConfig, logger) { * @type {String} */ this._version = version; - /** - * Boolean flag indicating whether or not the configuration instance was able - * to find a set of usable credentials to attempt authorization against the - * Stackdriver API. - * @memberof Configuration - * @private - * @type {Boolean} - */ - this._lacksValidCredentials = true; /** * The _givenConfiguration property holds a ConfigurationOptions object * which, if valid, will be merged against by the values taken from the meta- @@ -278,29 +267,6 @@ Configuration.prototype._gatherLocalConfiguration = function() { } else if (has(this._givenConfiguration, 'credentials')) { throw new Error('config.credentials must be a valid credentials object'); } - this._checkAuthConfiguration(); -}; -/** - * The _checkAuthConfiguration function is responsible for determining whether - * or not a configuration instance, after having gathered configuration from - * environment and runtime, has credentials information enough to attempt - * authorization with the Stackdriver Errors API. - * @memberof Configuration - * @private - * @function _checkAuthConfiguration - * @returns {Undefined} - does not return anything - */ -Configuration.prototype._checkAuthConfiguration = function() { - if (!isNull(this._key) || !isNull(this._keyFilename) || - !isNull(this._credentials) || - !isEmpty(process.env.GOOGLE_APPLICATION_CREDENTIALS)) { - this._lacksValidCredentials = false; - } else { - this._logger.warn([ - 'Unable to find credential information on instance. This library will', - 'be unable to communicate with the Stackdriver API to save errors.' - ].join(' ')); - } }; /** * The _checkLocalProjectId function is responsible for determing whether the @@ -408,14 +374,4 @@ Configuration.prototype.getServiceContext = function() { Configuration.prototype.getVersion = function() { return this._version; }; -/** - * Returns the _lacksValidCredentials property on the instance. - * @memberof Configuration - * @public - * @function getVersion - * @returns {Boolean} - returns the _lacksValidCredentials property - */ -Configuration.prototype.lacksCredentials = function() { - return this._lacksValidCredentials; -}; module.exports = Configuration; diff --git a/packages/error-reporting/src/interfaces/express.js b/packages/error-reporting/src/interfaces/express.js index 07a0614a603..07803c173fe 100644 --- a/packages/error-reporting/src/interfaces/express.js +++ b/packages/error-reporting/src/interfaces/express.js @@ -48,10 +48,6 @@ function makeExpressHandler(client, config) { var ctxService = ''; var ctxVersion = ''; - if (config.lacksCredentials()) { - next(err); - } - if (isObject(config)) { ctxService = config.getServiceContext().service; ctxVersion = config.getServiceContext().version; diff --git a/packages/error-reporting/src/interfaces/hapi.js b/packages/error-reporting/src/interfaces/hapi.js index 10f542c8863..9f8855a2853 100644 --- a/packages/error-reporting/src/interfaces/hapi.js +++ b/packages/error-reporting/src/interfaces/hapi.js @@ -78,26 +78,18 @@ function makeHapiPlugin(client, config) { if (isObject(server)) { if (isFunction(server.on)) { server.on('request-error', function(req, err) { - var em = hapiErrorHandler(req, err, config); - - if (!config.lacksCredentials()) { - client.sendError(em); - } + client.sendError(hapiErrorHandler(req, err, config)); }); } if (isFunction(server.ext)) { server.ext('onPreResponse', function(request, reply) { - var em = null; - if (isObject(request) && isObject(request.response) && request.response.isBoom) { - em = hapiErrorHandler(request, new Error(request.response.message), - config); - - if (!config.lacksCredentials()) { - client.sendError(em); - } + var em = hapiErrorHandler(request, + new Error(request.response.message), + config); + client.sendError(em); } if (isObject(reply) && isFunction(reply.continue)) { diff --git a/packages/error-reporting/src/interfaces/koa.js b/packages/error-reporting/src/interfaces/koa.js index e3f4751d068..b0297fb7eaf 100644 --- a/packages/error-reporting/src/interfaces/koa.js +++ b/packages/error-reporting/src/interfaces/koa.js @@ -41,18 +41,13 @@ function koaErrorHandler(client, config) { * @returns {Undefined} does not return anything */ return function *(next) { - var em; var svc = config.getServiceContext(); try { yield next; } catch (err) { - if (config.lacksCredentials()) { - return; - } - - em = new ErrorMessage() + var em = new ErrorMessage() .consumeRequestInformation( koaRequestInformationExtractor(this.request, this.response)) .setServiceContext(svc.service, diff --git a/packages/error-reporting/src/interfaces/manual.js b/packages/error-reporting/src/interfaces/manual.js index f2c7230158f..e2dbc74e495 100644 --- a/packages/error-reporting/src/interfaces/manual.js +++ b/packages/error-reporting/src/interfaces/manual.js @@ -56,9 +56,6 @@ function handlerSetup(client, config) { */ function reportManualError(err, request, additionalMessage, callback) { var em; - if (config.lacksCredentials()) { - return; - } if (isString(request)) { // no request given callback = additionalMessage; diff --git a/packages/error-reporting/src/interfaces/restify.js b/packages/error-reporting/src/interfaces/restify.js index df03996aae4..5d8a6e89d6f 100644 --- a/packages/error-reporting/src/interfaces/restify.js +++ b/packages/error-reporting/src/interfaces/restify.js @@ -128,9 +128,6 @@ function restifyRequestHandler(client, config, req, res, next) { function serverErrorHandler(client, config, server) { server.on('uncaughtException', function(req, res, reqConfig, err) { - if (config.lacksCredentials()) { - return; - } var em = new ErrorMessage().consumeRequestInformation( expressRequestInformationExtractor(req, res));