From cad0fa27ddf93b8fe4d528bc4e3be0f97d483cb8 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 17 Nov 2014 11:09:16 -0500 Subject: [PATCH 1/2] provide better error messaging - fixes #286 --- lib/common/util.js | 9 +++++++-- package.json | 2 +- test/common/util.js | 23 ++++++++++++++++++----- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/common/util.js b/lib/common/util.js index 819f1261258..c22fa9a4a79 100644 --- a/lib/common/util.js +++ b/lib/common/util.js @@ -359,9 +359,14 @@ function makeAuthorizedRequest(config) { if (err.code === 401 && ++tokenRefreshAttempts <= MAX_TOKEN_REFRESH_ATTEMPTS) { authorize(reqOpts, onAuthorizedRequest); - } else { - (callback.onAuthorized || callback)(err); + return; } + + if (err.message === 'SignFinal error') { + err.message = 'There was an error with your private key.'; + } + + (callback.onAuthorized || callback)(err); return; } diff --git a/package.json b/package.json index bde62b4199a..cb8e648779c 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "dependencies": { "duplexify": "^3.1.2", "extend": "^1.3.0", - "google-service-account": "^1.0.0", + "google-service-account": "^1.0.3", "mime": "^1.2.11", "node-uuid": "^1.4.1", "protobufjs": "^3.4.0", diff --git a/test/common/util.js b/test/common/util.js index f9fd0a234bb..0e6e031d2b7 100644 --- a/test/common/util.js +++ b/test/common/util.js @@ -20,6 +20,7 @@ var assert = require('assert'); var duplexify = require('duplexify'); +var gsa = require('google-service-account'); var request = require('request'); var util = require('sandboxed-module') @@ -31,20 +32,17 @@ var util = require('sandboxed-module') }); var gsa_Override; - function fakeGsa() { var args = [].slice.apply(arguments); - var results = (gsa_Override || util.noop).apply(null, args); + var results = (gsa_Override || gsa).apply(null, args); gsa_Override = null; return results || { getCredentials: util.noop }; } -var request_Cached = request; var request_Override; - function fakeRequest() { var args = [].slice.apply(arguments); - var results = (request_Override || request_Cached).apply(null, args); + var results = (request_Override || request).apply(null, args); request_Override = null; return results; } @@ -322,6 +320,21 @@ describe('common/util', function() { }); }); + it('should handle malformed key response', function(done) { + var makeRequest = util.makeAuthorizedRequest({ + credentials: { + client_email: 'invalid@email', + private_key: 'invalid-key' + } + }); + + makeRequest({}, function (err) { + var errorMessage = 'There was an error with your private key.'; + assert.equal(err.message, errorMessage); + done(); + }); + }); + it('should try to reconnect if token invalid', function(done) { var attempts = 0; var expectedAttempts = 2; From 0b0a61015d9237872ff3b441df531499e864482d Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 17 Nov 2014 14:45:12 -0500 Subject: [PATCH 2/2] more helpful error message https://github.com/GoogleCloudPlatform/gcloud-node/pull/303#discussion_r20447351 --- lib/common/util.js | 5 ++++- test/common/util.js | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/common/util.js b/lib/common/util.js index c22fa9a4a79..f12ca02f8bf 100644 --- a/lib/common/util.js +++ b/lib/common/util.js @@ -363,7 +363,10 @@ function makeAuthorizedRequest(config) { } if (err.message === 'SignFinal error') { - err.message = 'There was an error with your private key.'; + err.message = [ + 'Your private key is in an unexpected format and cannot be used.', + 'Please try again with another private key.' + ].join(' '); } (callback.onAuthorized || callback)(err); diff --git a/test/common/util.js b/test/common/util.js index 0e6e031d2b7..3cb82b85758 100644 --- a/test/common/util.js +++ b/test/common/util.js @@ -329,7 +329,10 @@ describe('common/util', function() { }); makeRequest({}, function (err) { - var errorMessage = 'There was an error with your private key.'; + var errorMessage = [ + 'Your private key is in an unexpected format and cannot be used.', + 'Please try again with another private key.' + ].join(' '); assert.equal(err.message, errorMessage); done(); });