Bug 738726 - Add more error checking to feeds to prevent restarts: 1. Add try catch to last throwable ios.newUri code remaining. 2. Workaround for bug 737966 regression. Fix up xhr parsing. 3. Force an msgDatabase reparse and continue feed processing, if not available. 4. Better logging. 5. Save untold downloading/parsing by fixing broken saving of Last-Modified timestamp. r=dbienvenu,a=Standard8
authoralta88 <alta88@gmail.com>
Fri, 06 Apr 2012 11:00:07 -0600
changeset 11251 17a96f4bde1222584ba4fc56217dae4930835f9d
parent 11250 9550e38058abe4b56dc01dc17257f326d347e551
child 11252 3ac782de680a8ebeb2ff9178b7fab9206ba0adfc
push id463
push userbugzilla@standard8.plus.com
push dateTue, 24 Apr 2012 17:34:51 +0000
treeherdercomm-beta@e53588e8f7b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbienvenu, Standard8
bugs738726, 737966
Bug 738726 - Add more error checking to feeds to prevent restarts: 1. Add try catch to last throwable ios.newUri code remaining. 2. Workaround for bug 737966 regression. Fix up xhr parsing. 3. Force an msgDatabase reparse and continue feed processing, if not available. 4. Better logging. 5. Save untold downloading/parsing by fixing broken saving of Last-Modified timestamp. r=dbienvenu,a=Standard8
mailnews/extensions/newsblog/content/Feed.js
mailnews/extensions/newsblog/content/feed-parser.js
mailnews/extensions/newsblog/content/utils.js
mailnews/extensions/newsblog/js/newsblog.js
--- a/mailnews/extensions/newsblog/content/Feed.js
+++ b/mailnews/extensions/newsblog/content/Feed.js
@@ -66,21 +66,26 @@ var FeedCache =
   {
     var index = this.normalizeHost(aUrl);
     if (index in this.mFeeds)
       delete this.mFeeds[index];
   },
 
   normalizeHost: function (aUrl)
   {
-    var ioService = Components.classes["@mozilla.org/network/io-service;1"].
-                    getService(Components.interfaces.nsIIOService);
-    var normalizedUrl = ioService.newURI(aUrl, null, null);
-    normalizedUrl.host = normalizedUrl.host.toLowerCase();
-    return normalizedUrl.spec;
+    try
+    {
+      let normalizedUrl = Services.io.newURI(aUrl, null, null);
+      normalizedUrl.host = normalizedUrl.host.toLowerCase();
+      return normalizedUrl.spec
+    }
+    catch (ex)
+    {
+      return aUrl;
+    }
   }
 };
 
 function Feed(aResource, aRSSServer) 
 {
   this.resource = aResource.QueryInterface(Components.interfaces.nsIRDFResource);
   this.server = aRSSServer;
 }
@@ -168,16 +173,18 @@ Feed.prototype =
                    .createInstance(Components.interfaces.nsIXMLHttpRequest);
     this.request.onprogress = this.onProgress; // must be set before calling .open
     this.request.open("GET", this.url, true);
 
     var lastModified = this.lastModified;
     if (lastModified)
       this.request.setRequestHeader("If-Modified-Since", lastModified);
 
+    // Only order what you're going to eat...
+    this.request.responseType = "document";
     this.request.overrideMimeType("text/xml");
     this.request.onload = this.onDownloaded;
     this.request.onerror = this.onDownloadError;
     FeedCache.putFeed(this);
     this.request.send(null);
   }, 
 
   onDownloaded: function(aEvent) 
@@ -189,23 +196,25 @@ Feed.prototype =
     {
       Feed.prototype.onDownloadError(aEvent);
       return;
     }
     var feed = FeedCache.getFeed(url);
     if (!feed)
       throw("error after downloading " + url + ": couldn't retrieve feed from request");
 
-    // if the request has a Last-Modified header on it, then go ahead and remember
-    // that as a property on the feed so we can use it when making future requests.    
+    // If the server sends a Last-Modified header, store the property on the
+    // feed so we can use it when making future requests, to avoid downloading
+    // and parsing feeds that have not changed.
     var lastModifiedHeader = request.getResponseHeader('Last-Modified');
     if (lastModifiedHeader)
-      this.lastModified = lastModifiedHeader;
+      feed.lastModified = lastModifiedHeader;
 
-    feed.parse(); // parse will asynchronously call the download callback when it is done
+    // The download callback is called asynchronously when parse() is done.
+    feed.parse();
   }, 
   
   onProgress: function(aEvent) 
   {
     var request = aEvent.target;
     var url = request.channel.originalURI.spec;
     var feed = FeedCache.getFeed(url);
 
@@ -215,34 +224,36 @@ Feed.prototype =
 
   onDownloadError: function(aEvent) 
   {
     var request = aEvent.target;
     var url = request.channel.originalURI.spec;
     var feed = FeedCache.getFeed(url);
     if (feed.downloadCallback) 
     {
-      // if the http status code is a 304, then the feed has not been modified since we last downloaded it.
       var error = kNewsBlogRequestFailure;
       try
       {
         if (request.status == 304)
+          // If the http status code is 304, the feed has not been modified
+          // since we last downloaded it and does not need to be parsed.
           error = kNewsBlogNoNewItems;
       } catch (ex) {}
       feed.downloadCallback.downloaded(feed, error);
     }
     
     FeedCache.removeFeed(url);
   },
 
   onParseError: function(aFeed) 
   {
     if (aFeed)
     {
       aFeed.mInvalidFeed = true;
+      aFeed.lastModified = "";
 
       if (aFeed.downloadCallback)
         aFeed.downloadCallback.downloaded(aFeed, kNewsBlogInvalidFeed);
 
       FeedCache.removeFeed(aFeed.url);
     }
   },
 
@@ -348,31 +359,21 @@ Feed.prototype =
     if (old_link)
       ds.Change(this.resource, RSS_LINK, old_link, aNewLink);
     else
       ds.Assert(this.resource, RSS_LINK, aNewLink, true);
   },
 
   parse: function() 
   {
-    // Figures out what description language (RSS, Atom) and version this feed
-    // is using and calls a language/version-specific feed parser.
-
-    debug("parsing feed " + this.url);
-
-    if (!this.request.responseText) 
-    {
-      this.onParseError(this);
-      return;
-    }
-      
-    // create a feed parser which will parse the feed for us
+    // Create a feed parser which will parse the feed.
     var parser = new FeedParser();
-    this.itemsToStore = parser.parseFeed(this, this.request.responseText, this.request.responseXML, this.request.channel.URI);
-  
+    this.itemsToStore = parser.parseFeed(this,
+                                         this.request.responseXML,
+                                         this.request.channel.URI);
     if (this.mInvalidFeed)
     {
       this.request = null;
       this.mInvalidFeed = false;
       return;
     }
 
     // storeNextItem will iterate through the parsed items, storing each one.
--- a/mailnews/extensions/newsblog/content/feed-parser.js
+++ b/mailnews/extensions/newsblog/content/feed-parser.js
@@ -44,21 +44,23 @@ var gIOService = Components.classes["@mo
 function FeedParser() 
 {}
 
 FeedParser.prototype = 
 {
   // parseFeed returns an array of parsed items ready for processing
   // it is currently a synchronous operation. If there was an error parsing the feed, 
   // parseFeed returns an empty feed in addition to calling aFeed.onParseError
-  parseFeed: function (aFeed, aSource, aDOM, aBaseURI)
+  parseFeed: function (aFeed, aDOM, aBaseURI)
   {
-    if (!aSource || !(aDOM instanceof Components.interfaces.nsIDOMXMLDocument))
+    if (!(aDOM instanceof Components.interfaces.nsIDOMXMLDocument) ||
+        aDOM.documentElement.getElementsByTagNameNS("http://www.mozilla.org/newlayout/xml/parsererror.xml", "parsererror")[0])
     {
-      aFeed.onParseError(aFeed);   
+      // No xml doc or gecko caught a basic parsing error.
+      aFeed.onParseError(aFeed);
       return new Array();
     }
     else if((aDOM.documentElement.namespaceURI == "http://www.w3.org/1999/02/22-rdf-syntax-ns#")
             && (aDOM.documentElement.getElementsByTagNameNS("http://purl.org/rss/1.0/", "channel")[0]))
     {
       debug(aFeed.url + " is an RSS 1.x (RDF-based) feed");
       // aSource can be misencoded (XMLHttpRequest converts to UTF-8 by default), 
       // but the DOM is almost always right because it uses the hints in the XML file.
@@ -73,17 +75,17 @@ FeedParser.prototype =
       debug(aFeed.url + " is an Atom 0.3 feed");
       return this.parseAsAtom(aFeed, aDOM);
     }
     else if (aDOM.documentElement.namespaceURI == ATOM_IETF_NS)
     {
       debug(aFeed.url + " is an IETF Atom feed");
       return this.parseAsAtomIETF(aFeed, aDOM);
     }
-    else if (aSource.search(/"http:\/\/my\.netscape\.com\/rdf\/simple\/0\.9\/"/) != -1)
+    else if (aDOM.documentElement.getElementsByTagNameNS("http://my.netscape.com/rdf/simple/0.9/", "channel")[0])
     {
       debug(aFeed.url + " is an 0.90 feed");
       return this.parseAsRSS2(aFeed, aDOM);
     }
     // XXX Explicitly check for RSS 2.0 instead of letting it be handled by the
     // default behavior (who knows, we may change the default at some point).
     else 
     {
--- a/mailnews/extensions/newsblog/content/utils.js
+++ b/mailnews/extensions/newsblog/content/utils.js
@@ -91,18 +91,19 @@ var FeedUtils = {
         }
       }
     },
 
     downloaded: function(feed, aErrorCode)
     {
       FeedUtils.log.debug("downloaded: "+
                           (this.mSubscribeMode ? "Subscribe " : "Update ") +
-                          "feed:errorCode - " +
-                          feed.name+" : "+aErrorCode);
+                          "errorCode:feed:folder - " +
+                          aErrorCode+" : "+feed.name+" : "+
+                          (feed.folder ? feed.folder.filePath.path : "null"));
       if (this.mSubscribeMode)
       {
         if (aErrorCode == FeedUtils.kNewsBlogSuccess)
         {
           // If we get here we should always have a folder by now, either in
           // feed.folder or FeedItems created the folder for us.
           updateFolderFeedUrl(feed.folder, feed.url, false);
 
@@ -125,29 +126,43 @@ var FeedUtils = {
         }
       }
 
       if (feed.folder && aErrorCode != FeedUtils.kNewsBlogFeedIsBusy)
         // Free msgDatabase after new mail biff is set; if busy let the next
         // result do the freeing.  Otherwise new messages won't be indicated.
         feed.folder.msgDatabase = null;
 
+      let message = "";
+      switch (aErrorCode) {
+        case FeedUtils.kNewsBlogSuccess:
+        case FeedUtils.kNewsBlogFeedIsBusy:
+          break;
+        case FeedUtils.kNewsBlogNoNewItems:
+          message = feed.url+". " +
+                    FeedUtils.strings.GetStringFromName(
+                      "newsblog-noNewArticlesForFeed");
+          break;
+        case FeedUtils.kNewsBlogInvalidFeed:
+          message = FeedUtils.strings.formatStringFromName(
+                      "newsblog-feedNotValid", [feed.url], 1);
+          break;
+        case FeedUtils.kNewsBlogRequestFailure:
+          message = FeedUtils.strings.formatStringFromName(
+                      "newsblog-networkError", [feed.url], 1);
+          break;
+      }
+      if (message)
+        FeedUtils.log.info("downloaded: "+
+                           (this.mSubscribeMode ? "Subscribe " : "Update ") +
+                           message);
+
       if (this.mStatusFeedback)
       {
-        if (aErrorCode == FeedUtils.kNewsBlogNoNewItems)
-          this.mStatusFeedback.showStatusString(
-            FeedUtils.strings.GetStringFromName("newsblog-noNewArticlesForFeed"));
-        else if (aErrorCode == FeedUtils.kNewsBlogInvalidFeed)
-          this.mStatusFeedback.showStatusString(
-            FeedUtils.strings.formatStringFromName("newsblog-feedNotValid",
-                                                   [feed.url], 1));
-        else if (aErrorCode == FeedUtils.kNewsBlogRequestFailure)
-          this.mStatusFeedback.showStatusString(
-            FeedUtils.strings.formatStringFromName("newsblog-networkError",
-                                                   [feed.url], 1));
+        this.mStatusFeedback.showStatusString(message);
         this.mStatusFeedback.stopMeteors();
       }
 
       if (!--this.mNumPendingFeedDownloads)
       {
         this.mFeeds = {};
         this.mSubscribeMode = false;
 
--- a/mailnews/extensions/newsblog/js/newsblog.js
+++ b/mailnews/extensions/newsblog/js/newsblog.js
@@ -72,46 +72,59 @@ var nsNewsBlogFeedDownloader =
     // Add the base folder; it does not get added by ListDescendents.
     allFolders.AppendElement(aFolder);
     aFolder.ListDescendents(allFolders);
     let numFolders = allFolders.Count();
     let trashFolder =
         aFolder.rootFolder.getFolderWithFlags(Ci.nsMsgFolderFlags.Trash);
 
     function feeder() {
+      let folder;
       for (let i = 0; i < numFolders; i++) {
-        let folder = allFolders.GetElementAt(i).QueryInterface(Ci.nsIMsgFolder);
+        folder = allFolders.GetElementAt(i).QueryInterface(Ci.nsIMsgFolder);
         FeedUtils.log.debug("downloadFeed: START x/# foldername:uri - "+
                             (i+1)+"/"+ numFolders+" "+
                             folder.name+":"+folder.URI);
+
+        // Ensure msgDatabase for the folder is open for new message processing.
+        let msgDb;
+        try {
+          msgDb = folder.msgDatabase;
+        }
+        catch (ex) {}
+        if (!msgDb) {
+          // Force a reparse.  After the async reparse the folder will be ready
+          // for the next cycle; don't bother with a listener.  Continue with
+          // the next folder, as attempting to add a message to a folder with
+          // an unavailable msgDatabase will throw later.
+          FeedUtils.log.debug("downloadFeed: rebuild msgDatabase for "+
+                              folder.name+" - "+folder.filePath.path);
+          try
+          {
+            // Ignore error returns.
+            folder.QueryInterface(Ci.nsIMsgLocalMailFolder).
+                   getDatabaseWithReparse(null, null);
+          }
+          catch (ex) {}
+          continue;
+        }
+
         let feedUrlArray = getFeedUrlsInFolder(folder);
         // Continue if there are no feedUrls for the folder in the feeds
         // database.  All folders in Trash are now unsubscribed, so perhaps
         // we may not want to check that here each biff each folder.
         if (!feedUrlArray ||
             (aFolder.isServer && trashFolder && trashFolder.isAncestorOf(folder)))
           continue;
 
         FeedUtils.log.debug("downloadFeed: CONTINUE foldername:urlArray - "+
                             folder.name+":"+feedUrlArray);
 
         FeedUtils.progressNotifier.init(aMsgWindow, false);
 
-        // Ensure msgDatabase for the folder is open for new message processing.
-        let msgDb;
-        try {
-          msgDb = aFolder.msgDatabase;
-        }
-        catch (ex) {}
-        if (!msgDb) {
-          // Forcing a reparse with listener here is the last resort.  This is
-          // not implemented; it is currently not done and may be unnecessary
-          // given other msgDatabase sync fixes.
-        }
-
         // We need to kick off a download for each feed.
         let id, feed;
         for (let url in feedUrlArray)
         {
           if (feedUrlArray[url])
           {
             id = rdf.GetResource(feedUrlArray[url]);
             feed = new Feed(id, folder.server);
@@ -122,34 +135,48 @@ var nsNewsBlogFeedDownloader =
             FeedUtils.log.debug("downloadFeed: DOWNLOAD feed url - "+
                                 feedUrlArray[url]);
           }
 
           Services.tm.mainThread.dispatch(function() {
             try {
               getFeed.next();
             }
-            catch (ex if ex instanceof StopIteration) {
-              // Finished with all feeds in base folder and its subfolders.
-              FeedUtils.log.debug("downloadFeed: StopIteration");
+            catch (ex) {
+              if (ex instanceof StopIteration)
+                // Finished with all feeds in base folder and its subfolders.
+                FeedUtils.log.debug("downloadFeed: Finished with folder - "+
+                                    aFolder.name);
+              else
+              {
+                FeedUtils.log.error("downloadFeed: error - " + ex);
+                FeedUtils.progressNotifier.downloaded({name: folder.name}, 0);
+              }
             }
           }, Ci.nsIThread.DISPATCH_NORMAL);
 
           yield;
         }
       }
     }
 
     let getFeed = feeder();
     try {
       getFeed.next();
-    } 
-    catch (ex if ex instanceof StopIteration) {
-      // Nothing to do.
-      FeedUtils.log.debug("downloadFeed: Nothing to do");
+    }
+    catch (ex) {
+      if (ex instanceof StopIteration)
+        // Nothing to do.
+        FeedUtils.log.debug("downloadFeed: Nothing to do in folder - "+
+                            aFolder.name);
+      else
+      {
+        FeedUtils.log.error("downloadFeed: error - " + ex);
+        FeedUtils.progressNotifier.downloaded({name: aFolder.name}, 0);
+      }
     }
   },
 
   subscribeToFeed: function(aUrl, aFolder, aMsgWindow)
   {
     if (!gExternalScriptsLoaded)
       loadScripts();