Fix failed assertion for unknown app server errors #6783
Conversation
michael-wb
changed the title
michael-wb
force-pushed
the
mwb/handle-unknown-app-errors
branch
from
4c16d3b to
b3ac96c
Compare
michael-wb
changed the title
michael-wb
force-pushed
the
mwb/handle-unknown-app-errors
branch
from
b3ac96c to
95fc4c9
Compare
src/realm/error_codes.cpp
Outdated
| @@ -88,7 +88,7 @@ ErrorCategory ErrorCodes::error_categories(Error code) | |||
| case StaleAccessor: | |||
| case WrongThread: | |||
| case WrongTransactionState: | |||
| return ErrorCategory().set(ErrorCategory::logic_error); | |||
There was a problem hiding this comment.
Are these actually runtime errors? Would it make more sense to just make AppError inherit from Exception rather than adding basically all other kinds of errors to RuntimeError?
There was a problem hiding this comment.
I reverted these changes and made AppError inherit from Exception like you recommended.
I kept the AppServiceError error code as a way to report App errors that the client doesn't know about.
src/realm/error_codes.cpp
Outdated
| .set(ErrorCategory::runtime_error) | ||
| .set(ErrorCategory::app_error) | ||
| .set(ErrorCategory::custom_error); | ||
| return ErrorCategory().set(ErrorCategory::app_error).set(ErrorCategory::custom_error); |
There was a problem hiding this comment.
Sorry, I must have not been clear, in my past comment I was suggesting just not changing any of the existing error categories or which codes fall into what but making the AppError inherit from Exception so it doesn't have to only be RuntimeErrors.
There was a problem hiding this comment.
Sorry, I thought I was reverting the changes back. I'll fix these.
| REALM_ASSERT(ErrorCodes::error_categories(code).test(ErrorCategory::app_error)); | ||
| } | ||
|
|
||
| AppError::AppError(std::string server_err, std::string message, std::string link, |
There was a problem hiding this comment.
you could make server_err optional and then have only one constructor
What, How & Why?
If an unknown error is received from the server, the current
AppUtils::check_for_errors()function will attempt to create anAppErrorusing theErrorCodes::UnknownErrorvalue, which does not have the runtime_error or app_error error category, leading to an assertion failure when the AppError is constructed.This code updates the error code value when
UnknownErroris returned from theErrorCodes::from_string()function to use the "generic"ErrorCodes::AppServerError. Theserver_errorstring value was added to theAppErrorclass for storing the original error code string returned from the server.Added an AppError test to cover all the paths for creating an AppError in
AppUtils::check_for_errors().In a separate PR, the plan is to update this function so all errors returned from the server will create an AppError with code AppServerError and the specific error details will be contained in the "server_error" and "reason" strings.
Fixes #6758
☑️ ToDos
[ ] C-API, if public C++ API changed.