Closed Bug 497809 Opened 15 years ago Closed 10 years ago

non-sane feeditems.rdf not recognized/deleted/rebuilt

Categories

(MailNews Core :: Feed Reader, enhancement)

x86_64
Other
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: peterhollin, Assigned: alta88)

References

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10 (.NET CLR 3.5.30729)
Build Identifier: 2.0.0.21

Feeditems.rdf became corrupted.  Can't tell how corruption.  Thunderbird spinner continues to run after "Checking for new RSS items" and no items are downloaded.  Others have encountered the problem as described in http://forums.mozillazine.org/viewtopic.php?f=39&t=200962&p=1159281 .  I decided to investigate further, and found the corrupted feeditems.rdf .  Renamed feeditems.rdf and thunderbird rebuilt it and everything seems fine.

Reproducible: Always

Steps to Reproduce:
1.  Break feeditems.rdf
2.  Attempt to download rss feeds
3.  See that Thunderbird reports no problem and spins.


Expected Results:  
Thunderbird should throw an error message when an RDF database is corrupt or can't pass a validation.  This could be particularly problematic in the case of gradual, creeping corruptions where the data lost increases over time.  Doing so would increase the usability and perceived reliability of thunderbird.  Ideally, it should generate a crash report so that developers are informed when it happens so that patterns may be identified in database corruptions.  If you really want to get fancy, thunderbird can/should automatically attempt to rebuild the file(s) in question and/or automatically delete files which are not particularly critical.

The RDF file in question consisted entirely of blank or unprintable characters, but the corruption is not really the focus since if could have been a one-time event not caused by thunderbird.  The lack of validation is the issue I want to address.
Component: General → Feed Reader
Product: Thunderbird → MailNews Core
QA Contact: general → feed.reader
Attached patch feeditems.patch (obsolete) — Splinter Review
This patch will, once in a session at startup/first use, check that a feed account's feeditems.rdf is valid XML; if not, the bad file will be renamed and a new one created.
Assignee: nobody → alta88
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8356139 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8356139 [details] [diff] [review]
feeditems.patch

Review of attachment 8356139 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! r=mkmelin

::: mailnews/extensions/newsblog/content/FeedUtils.jsm
@@ +564,5 @@
> +    if (dom instanceof Ci.nsIDOMXMLDocument &&
> +        dom.documentElement.namespaceURI != this.MOZ_PARSERERROR_NS)
> +      return file;
> +
> +    // Error on the file. Rename it and create a new one.

in the file, no? same on the next row

@@ +569,5 @@
> +    this.log.debug("FeedUtils.getItemsFile: error on feeditems.rdf");
> +    let errName, dir, i = 1;
> +    while (i <= 10) {
> +      dir = file.parent.clone();
> +      errName = "feeditems_Error_" + (i++) + ".rdf";

maybe it would be safer to use a timestamp, like
(new Date().toISOString()).replace(/\D/g, "")

... in case it happens more frequently to someone

I'd also lowercase Error
Attachment #8356139 - Flags: review?(mkmelin+mozilla) → review+
Attached patch feeditems.patchSplinter Review
Attachment #8356139 - Attachment is obsolete: true
Attachment #8357367 - Flags: review+
(In reply to Magnus Melin from comment #3)
> Comment on attachment 8356139 [details] [diff] [review]
> feeditems.patch
> 
> Review of attachment 8356139 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me! r=mkmelin
> 
> ::: mailnews/extensions/newsblog/content/FeedUtils.jsm
> @@ +564,5 @@
> > +    if (dom instanceof Ci.nsIDOMXMLDocument &&
> > +        dom.documentElement.namespaceURI != this.MOZ_PARSERERROR_NS)
> > +      return file;
> > +
> > +    // Error on the file. Rename it and create a new one.
> 
> in the file, no? same on the next row
> 

i'd say there's a subtle difference in en; on means more 'with' and in is more specific.  but ok.

> @@ +569,5 @@
> > +    this.log.debug("FeedUtils.getItemsFile: error on feeditems.rdf");
> > +    let errName, dir, i = 1;
> > +    while (i <= 10) {
> > +      dir = file.parent.clone();
> > +      errName = "feeditems_Error_" + (i++) + ".rdf";
> 
> maybe it would be safer to use a timestamp, like
> (new Date().toISOString()).replace(/\D/g, "")
> 
> ... in case it happens more frequently to someone
> 
> I'd also lowercase Error

yeah, date is much simpler/better.  the reasons for past frequent corruption are mostly gone, so this should be a rare case, though there is still core rdf (and cache) fragility in crash cases.

these 2 dbs should really be moved out of rdf.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/32dea0094b41
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in before you can comment on or make changes to this bug.