Open
Bug 1387775
Opened 7 years ago
Updated 2 years ago
Sqlite.jsm `onRow` handler swallows exceptions
Categories
(Toolkit :: Storage, enhancement, P3)
Toolkit
Storage
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox57 | --- | fix-optional |
People
(Reporter: lina, Unassigned, Mentored)
Details
(Whiteboard: [lang=cpp][good next bug])
I spent some time debugging why the tree structure I was building from a Places query was incomplete. It turns out one of the methods I was calling from the `onRow` callback had a bug, causing it to throw. (There are cases where I still want that method to throw, but this wasn't one of them). I didn't have SQLite logging turned on, so I missed the "Exception when calling onRow callback" logging: https://searchfox.org/mozilla-central/rev/39ccebaf1890f8731d534b5eb3818b66d2b25221/toolkit/modules/Sqlite.jsm#778-780,787 IIUC, removing the `try...catch` isn't enough. XPCOM will convert the exception into a failed `nsresult`, but https://searchfox.org/mozilla-central/rev/39ccebaf1890f8731d534b5eb3818b66d2b25221/storage/mozStorageAsyncStatementExecution.cpp#441,450,452 ignores it. I wonder if we could have `notifyResultsOnCallingThread` call `notifyErrorOnCallingThread`, cancel the statement, and eventually call `handleCompletion` with `REASON_ERROR`. Or, if it's not worth the trouble, I can remove the `onRow` handler from my code and iterate over the array afterward.
Comment 1•7 years ago
|
||
Now that we're targeting 57 and extensions can no longer access mozStorage, this sounds like a win with no downsides. I'm certainly on board with having all callbacks propagate errors forward. The main thing is to keep the error propagation exclusively on the owning/main thread (so we don't have to synchronize with I/O on the async thread and can otherwise maintain correctness).
Updated•7 years ago
|
status-firefox57:
--- → fix-optional
Priority: -- → P3
Updated•3 years ago
|
Mentor: mak
Whiteboard: [lang=cpp][good next bug]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•