Open Bug 1387775 Opened 3 years ago Updated 3 years ago
.jsm `on Row` handler swallows exceptions
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.
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).
You need to log in before you can comment on or make changes to this bug.