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

RESOLVED FIXED in Thunderbird 29.0

Status

--
enhancement
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: peterhollin, Assigned: alta88)

Tracking

unspecified
Thunderbird 29.0
x86_64
Other

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.82 KB, patch
alta88
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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.
(Assignee)

Updated

7 years ago
Component: General → Feed Reader
Product: Thunderbird → MailNews Core
QA Contact: general → feed.reader
(Assignee)

Updated

5 years ago
Duplicate of this bug: 952792
(Assignee)

Comment 2

5 years ago
Created attachment 8356139 [details] [diff] [review]
feeditems.patch


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+
(Assignee)

Comment 4

5 years ago
Created attachment 8357367 [details] [diff] [review]
feeditems.patch
Attachment #8356139 - Attachment is obsolete: true
Attachment #8357367 - Flags: review+
(Assignee)

Comment 5

5 years ago
(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
Last Resolved: 5 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.