From b7b6981ce24acf6dee454241f3e0c1d0a6deb85e Mon Sep 17 00:00:00 2001
From: Richard Newman <rnewman@mozilla.com>
Date: Mon, 5 Nov 2012 17:31:12 -0800
Subject: [PATCH] Bug 808750 - More logging and safety in resource callbacks.
 r=gps

---
 services/sync/modules/resource.js | 55 ++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js
index 457b92e35da5..d318f446b389 100644
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -13,10 +13,11 @@ const Cr = Components.results;
 const Cu = Components.utils;
 
 Cu.import("resource://services-common/async.js");
-Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-common/log4moz.js");
 Cu.import("resource://services-common/observers.js");
 Cu.import("resource://services-common/preferences.js");
-Cu.import("resource://services-common/log4moz.js");
+Cu.import("resource://services-common/utils.js");
+Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/util.js");
 
 const DEFAULT_LOAD_FLAGS =
@@ -115,7 +116,7 @@ AsyncResource.prototype = {
   },
   set uri(value) {
     if (typeof value == 'string')
-      this._uri = Utils.makeURI(value);
+      this._uri = CommonUtils.makeURI(value);
     else
       this._uri = value;
   },
@@ -264,9 +265,9 @@ AsyncResource.prototype = {
     } catch(ex) {
       // Got a response, but an exception occurred during processing.
       // This shouldn't occur.
-      this._log.warn("Caught unexpected exception " + Utils.exceptionStr(ex) +
+      this._log.warn("Caught unexpected exception " + CommonUtils.exceptionStr(ex) +
                      " in _onComplete.");
-      this._log.debug(Utils.stackTrace(ex));
+      this._log.debug(CommonUtils.stackTrace(ex));
     }
 
     // Process headers. They can be empty, or the call can otherwise fail, so
@@ -292,9 +293,9 @@ AsyncResource.prototype = {
         Observers.notify("weave:service:quota:remaining",
                          parseInt(headers["x-weave-quota-remaining"], 10));
     } catch (ex) {
-      this._log.debug("Caught exception " + Utils.exceptionStr(ex) +
+      this._log.debug("Caught exception " + CommonUtils.exceptionStr(ex) +
                       " visiting headers in _onComplete.");
-      this._log.debug(Utils.stackTrace(ex));
+      this._log.debug(CommonUtils.stackTrace(ex));
     }
 
     let ret     = new String(data);
@@ -309,7 +310,7 @@ AsyncResource.prototype = {
       try {
         return JSON.parse(ret);
       } catch (ex) {
-        this._log.warn("Got exception parsing response body: \"" + Utils.exceptionStr(ex));
+        this._log.warn("Got exception parsing response body: \"" + CommonUtils.exceptionStr(ex));
         // Stringify to avoid possibly printing non-printable characters.
         this._log.debug("Parse fail: Response body starts: \"" +
                         JSON.stringify((ret + "").slice(0, 100)) +
@@ -509,14 +510,21 @@ ChannelListener.prototype = {
   },
 
   onDataAvailable: function Channel_onDataAvail(req, cb, stream, off, count) {
-    let siStream = Cc["@mozilla.org/scriptableinputstream;1"].
-      createInstance(Ci.nsIScriptableInputStream);
-    siStream.init(stream);
+    let siStream;
+    try {
+      siStream = Cc["@mozilla.org/scriptableinputstream;1"].createInstance(Ci.nsIScriptableInputStream);
+      siStream.init(stream);
+    } catch (ex) {
+      this._log.warn("Exception creating nsIScriptableInputStream." + CommonUtils.exceptionStr(ex));
+      this._log.debug("Parameters: " + req.URI.spec + ", " + stream + ", " + off + ", " + count);
+      // Cannot proceed, so rethrow and allow the channel to cancel itself.
+      throw ex;
+    }
+
     try {
       this._data += siStream.read(count);
     } catch (ex) {
-      this._log.warn("Exception thrown reading " + count +
-                     " bytes from " + siStream + ".");
+      this._log.warn("Exception thrown reading " + count + " bytes from " + siStream + ".");
       throw ex;
     }
 
@@ -525,7 +533,7 @@ ChannelListener.prototype = {
     } catch (ex) {
       this._log.warn("Got exception calling onProgress handler during fetch of "
                      + req.URI.spec);
-      this._log.debug(Utils.exceptionStr(ex));
+      this._log.debug(CommonUtils.exceptionStr(ex));
       this._log.trace("Rethrowing; expect a failure code from the HTTP channel.");
       throw ex;
     }
@@ -534,10 +542,14 @@ ChannelListener.prototype = {
   },
 
   /**
-   * Create or push back the abort timer that kills this request
+   * Create or push back the abort timer that kills this request.
    */
   delayAbort: function delayAbort() {
-    Utils.namedTimer(this.abortRequest, this._timeout, this, "abortTimer");
+    try {
+      CommonUtils.namedTimer(this.abortRequest, this._timeout, this, "abortTimer");
+    } catch (ex) {
+      this._log.warn("Got exception extending abort timer: " + CommonUtils.exceptionStr(ex));
+    }
   },
 
   abortRequest: function abortRequest() {
@@ -597,8 +609,9 @@ ChannelNotificationListener.prototype = {
   asyncOnChannelRedirect:
     function asyncOnChannelRedirect(oldChannel, newChannel, flags, callback) {
 
-    this._log.debug("Channel redirect: " + oldChannel.URI.spec + ", " +
-                    newChannel.URI.spec + ", " + flags);
+    let oldSpec = (oldChannel && oldChannel.URI) ? oldChannel.URI.spec : "<undefined>";
+    let newSpec = (newChannel && newChannel.URI) ? newChannel.URI.spec : "<undefined>";
+    this._log.debug("Channel redirect: " + oldSpec + ", " + newSpec + ", " + flags);
 
     // For internal redirects, copy the headers that our caller set.
     try {
@@ -617,6 +630,10 @@ ChannelNotificationListener.prototype = {
     }
 
     // We let all redirects proceed.
-    callback.onRedirectVerifyCallback(Cr.NS_OK);
+    try {
+      callback.onRedirectVerifyCallback(Cr.NS_OK);
+    } catch (ex) {
+      this._log.error("onRedirectVerifyCallback threw!" + CommonUtils.exceptionStr(ex));
+    }
   }
 };