search

realm / realm-core

Core database component for the Realm Mobile Database SDKs
https://realm.io
Apache License 2.0
1.02k stars 165 forks source link

Make exception_to_status not abort #8009

Closed jedelbo closed 2 months ago

jedelbo commented 2 months ago

What, How & Why?

Probably better

☑️ ToDos

coveralls-official[bot] commented 2 months ago

Pull Request Test Coverage Report for Build jorgen.edelbo_422

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/realm/exceptions.cpp 0 1 0.0%
<!-- Total: 0 1 0.0% -->
Files with Coverage Reduction New Missed Lines %
src/realm/array_backlink.cpp 1 91.38%
src/realm/array_string.cpp 1 88.03%
src/realm/dictionary.cpp 1 85.16%
src/realm/sort_descriptor.cpp 1 94.06%
src/realm/util/file.cpp 1 84.84%
src/realm/util/serializer.cpp 1 90.43%
test/test_index_string.cpp 1 93.48%
src/realm/table_view.cpp 2 92.99%
src/realm/sync/noinst/protocol_codec.hpp 3 75.74%
src/realm/table.cpp 3 90.29%
<!-- Total: 79 -->
Totals Coverage Status
Change from base Build 2589: -0.02%
Covered Lines: 217315
Relevant Lines: 238555
💛 - Coveralls
jbreams commented 2 months ago

Having this condition being unreachable was a deliberate design choice so when totally unknown exception types like this happened we would at least get a stacktrace of where they were generated or rethrown. This has come up recently because NSError sometimes gets thrown from within the swift SDK. I think we should just add some support for converting NSError here or in the swift SDK rather than silently discarding all unknown exceptions. It seems like this change would make unknown exception types much harder to debug.

nicola-cab commented 2 months ago

Probably worth setting the message string for the NSError that gets thrown. It seems a similar scenario we had to deal with for setting client reset callback errors happening in the SDK. We have had 2 crash reports so far ...

jedelbo commented 2 months ago

Probably worth setting the message string for the NSError that gets thrown.

The problem is that we cannot catch an NSError in our code. It should have been translated back to an Exception in the Swift SDK, I think.

jedelbo commented 2 months ago

We probably have to leave it as it is.