Bug 976393 - ensure authentication errors log as much detail as possible. r=jedp
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 03 Mar 2014 09:55:30 +1100
changeset 171314 6a9f8ca4c9b1f4c26e526dde27fb520181d9606a
parent 171313 2ed56735cb6af92b1f7dd80dae1b084fa1e1fa24
child 171315 7378692291b756ddda6ed1f981754b1243da2e4e
push id26323
push userttaubert@mozilla.com
push dateMon, 03 Mar 2014 08:30:54 +0000
treeherdermozilla-central@4cb766685b73 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjedp
bugs976393
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 976393 - ensure authentication errors log as much detail as possible. r=jedp
services/common/tokenserverclient.js
services/sync/modules/browserid_identity.js
--- a/services/common/tokenserverclient.js
+++ b/services/common/tokenserverclient.js
@@ -29,31 +29,40 @@ const Prefs = new Preferences("services.
  *        (string) Error message.
  */
 this.TokenServerClientError = function TokenServerClientError(message) {
   this.name = "TokenServerClientError";
   this.message = message || "Client error.";
 }
 TokenServerClientError.prototype = new Error();
 TokenServerClientError.prototype.constructor = TokenServerClientError;
+TokenServerClientError.prototype._toStringFields = function() {
+  return {message: this.message};
+}
+TokenServerClientError.prototype.toString = function() {
+  return this.name + "(" + JSON.stringify(this._toStringFields()) + ")";
+}
 
 /**
  * Represents a TokenServerClient error that occurred in the network layer.
  *
  * @param error
  *        The underlying error thrown by the network layer.
  */
 this.TokenServerClientNetworkError =
  function TokenServerClientNetworkError(error) {
   this.name = "TokenServerClientNetworkError";
   this.error = error;
 }
 TokenServerClientNetworkError.prototype = new TokenServerClientError();
 TokenServerClientNetworkError.prototype.constructor =
   TokenServerClientNetworkError;
+TokenServerClientNetworkError.prototype._toStringFields = function() {
+  return {error: this.error};
+}
 
 /**
  * Represents a TokenServerClient error that occurred on the server.
  *
  * This type will be encountered for all non-200 response codes from the
  * server. The type of error is strongly enumerated and is stored in the
  * `cause` property. This property can have the following string values:
  *
@@ -77,24 +86,39 @@ TokenServerClientNetworkError.prototype.
  *   general -- A general server error has occurred. Clients should
  *     interpret this as an opaque failure.
  *
  * @param message
  *        (string) Error message.
  */
 this.TokenServerClientServerError =
  function TokenServerClientServerError(message, cause="general") {
+  this.now = new Date().toISOString(); // may be useful to diagnose time-skew issues.
   this.name = "TokenServerClientServerError";
   this.message = message || "Server error.";
   this.cause = cause;
 }
 TokenServerClientServerError.prototype = new TokenServerClientError();
 TokenServerClientServerError.prototype.constructor =
   TokenServerClientServerError;
 
+TokenServerClientServerError.prototype._toStringFields = function() {
+  let fields = {
+    now: this.now,
+    message: this.message,
+    cause: this.cause,
+  };
+  if (this.response) {
+    fields.response_body = this.response.body;
+    fields.response_headers = this.response.headers;
+    fields.response_status = this.response.status;
+  }
+  return fields;
+};
+
 /**
  * Represents a client to the Token Server.
  *
  * http://docs.services.mozilla.com/token/index.html
  *
  * The Token Server supports obtaining tokens for arbitrary apps by
  * constructing URI paths of the form <app>/<app_version>. However, the service
  * discovery mechanism emphasizes the use of full URIs and tries to not force
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -50,21 +50,29 @@ function deriveKeyBundle(kB) {
   let bundle = new BulkKeyBundle();
   // [encryptionKey, hmacKey]
   bundle.keyPair = [out.slice(0, 32), out.slice(32, 64)];
   return bundle;
 }
 
 /*
   General authentication error for abstracting authentication
-  errors from multiple sources (e.g., from FxAccounts, TokenServer)
-    'message' is a string with a description of the error
+  errors from multiple sources (e.g., from FxAccounts, TokenServer).
+  details is additional details about the error - it might be a string, or
+  some other error object (which should do the right thing when toString() is
+  called on it)
 */
-function AuthenticationError(message) {
-  this.message = message || "";
+function AuthenticationError(details) {
+  this.details = details;
+}
+
+AuthenticationError.prototype = {
+  toString: function() {
+    return "AuthenticationError(" + this.details + ")";
+  }
 }
 
 this.BrowserIDManager = function BrowserIDManager() {
   // NOTE: _fxaService and _tokenServerClient are replaced with mocks by
   // the test suite.
   this._fxaService = fxAccounts;
   this._tokenServerClient = new TokenServerClient();
   // will be a promise that resolves when we are ready to authenticate
@@ -157,21 +165,21 @@ this.BrowserIDManager.prototype = {
           Svc.Prefs.set("firstSync", "resetClient");
           Services.obs.notifyObservers(null, "weave:service:setup-complete", null);
           Weave.Utils.nextTick(Weave.Service.sync, Weave.Service);
         }
       }).then(null, err => {
         this._shouldHaveSyncKeyBundle = true; // but we probably don't have one...
         this.whenReadyToAuthenticate.reject(err);
         // report what failed...
-        this._log.error("Background fetch for key bundle failed: " + err.message);
+        this._log.error("Background fetch for key bundle failed: " + err);
       });
       // and we are done - the fetch continues on in the background...
     }).then(null, err => {
-      this._log.error("Processing logged in account: " + err.message);
+      this._log.error("Processing logged in account: " + err);
     });
   },
 
   observe: function (subject, topic, data) {
     this._log.debug("observed " + topic);
     switch (topic) {
     case fxAccountsCommon.ONLOGIN_NOTIFICATION:
       this.initializeWithCurrentIdentity(true);
@@ -420,17 +428,17 @@ this.BrowserIDManager.prototype = {
     log.info("Fetching assertion and token from: " + tokenServerURI);
 
     function getToken(tokenServerURI, assertion) {
       log.debug("Getting a token");
       let deferred = Promise.defer();
       let cb = function (err, token) {
         if (err) {
           log.info("TokenServerClient.getTokenFromBrowserIDAssertion() failed with: " + err.message);
-          return deferred.reject(new AuthenticationError(err.message));
+          return deferred.reject(new AuthenticationError(err));
         } else {
           log.debug("Successfully got a sync token");
           return deferred.resolve(token);
         }
       };
 
       client.getTokenFromBrowserIDAssertion(tokenServerURI, assertion, cb, headers);
       return deferred.promise;
@@ -460,17 +468,17 @@ this.BrowserIDManager.prototype = {
         token.expiration = this._now() + (token.duration * 1000) * 0.80;
         return token;
       })
       .then(null, err => {
         // TODO: write tests to make sure that different auth error cases are handled here
         // properly: auth error getting assertion, auth error getting token (invalid generation
         // and client-state error)
         if (err instanceof AuthenticationError) {
-          this._log.error("Authentication error in _fetchTokenForUser: " + err.message);
+          this._log.error("Authentication error in _fetchTokenForUser: " + err);
           // Drop the sync key bundle, but still expect to have one.
           // This will arrange for us to be in the right 'currentAuthState'
           // such that UI will show the right error.
           this._shouldHaveSyncKeyBundle = true;
           this._syncKeyBundle = null;
           Weave.Status.login = this.currentAuthState;
           Services.obs.notifyObservers(null, "weave:service:login:error", null);
         }