Closed Bug 696483 Opened 8 years ago Closed 8 years ago

missing reporterror

Categories

(Toolkit :: Form Manager, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: espindola, Assigned: espindola)

Details

Attachments

(1 file, 1 obsolete file)

In nsFormHistory.js we have

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

this should reportError.
Attached patch Don't suppress all exceptions. (obsolete) — Splinter Review
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
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.