Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In nsFormHistory.js we have

       try { this.dbConnection.close(); } catch(e) {}

this should reportError.
Created attachment 581607 [details] [diff] [review]
Don't suppress all exceptions.

The only error I have seen in this part of the code was the connection never being created, so this patch replaces the try/catch with an if.

https://tbpl.mozilla.org/?tree=Try&rev=e0b356c9e3aa
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #581607 - Flags: review?(mak77)
Comment on attachment 581607 [details] [diff] [review]
Don't suppress all exceptions.

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

If close() throws, the next call to remove() will fail (since the database file is in use) and we won't be able to proceed with dbOpen() and dbInit() regardless.
The problem is whether the exception will be able to make the Error Console at least, or if it will just be hidden and unreported.
So, just in case, we may still want to also catch and reportError it, also keeping the if condition that looks correct.
Attachment #581626 - Flags: review?(mak77) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.