Page MenuHomePhabricator

Throw informative JSError when SQLite query via ORM fails
AbandonedPublic

Authored by marcin on Jul 27 2022, 4:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 29, 8:30 PM
Unknown Object (File)
Mon, Nov 18, 11:46 AM
Unknown Object (File)
Tue, Nov 5, 7:18 AM
Unknown Object (File)
Tue, Nov 5, 7:18 AM
Unknown Object (File)
Tue, Nov 5, 7:17 AM
Unknown Object (File)
Nov 1 2024, 3:37 PM
Unknown Object (File)
Nov 1 2024, 3:37 PM
Unknown Object (File)
Nov 1 2024, 3:14 PM

Details

Reviewers
tomek
atul
Summary

CommCoreModule::getAllMessagesSync and CommCoreModule::getALlThreadsSync execute database query on teh WorkerThread without error handling. If database operation fails we are left with uninformative application crash. Catching error thrown by sqlite orm and re-throwing it as JSError enables us to continue using application and see detailed information regarding the error.

Test Plan

Temporarily place some throw std::system_error in SQLiteQueryExecutor::getAllMessages(). See that application does not crash but some UI window is displayed with JavaScript error detail.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-1449
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested changes to this revision.Jul 28 2022, 6:33 AM
tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
166–175

Is this error propagated to the js? The method returns jsiMessages which is a new array and doesn't contain any info about the exception jsi::Array(rt, numMessages). Or maybe I'm missing something and it works as expected? Maybe calling set_exception stops the execution? - that happens in another thread, so I guess there might be something more to this.

This revision now requires changes to proceed.Jul 28 2022, 6:33 AM
marcin added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
166–175

Error is propagated to JavaScript, I tested it as described in the test plan.
Additionally documentation supports why it works: messageResult is a C++ promise https://en.cppreference.com/w/cpp/thread/promise. This type can contain either result as the object of certain type (set by set_value), or pointer to an exception (set by set_exception). Calling a get on a promise that contains pointer to an exception is equivalent to throw exception https://en.cppreference.com/w/cpp/thread/promise/set_exception. In other words if we call set_exception on a promise then calling get() on the promise throws the exception. Creating JSError and setting it in the promise happens on different thread but get() method is called on the sam thread the function executes so JSError is thrown on the same thread the getAllMessagesSync function executes.

tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
166–175

Thanks! That explains everything I wanted to know!

This revision is now accepted and ready to land.Jul 28 2022, 7:30 AM