Add missing protocol errors#7982
Conversation
michael-wb
added
the
no-jira-ticket
| {"SyncConnectFailed", ErrorCodes::SyncConnectFailed}, | ||
| {"SyncConnectTimeout", ErrorCodes::SyncConnectTimeout}, | ||
| {"SyncInvalidSchemaChange", ErrorCodes::SyncInvalidSchemaChange}, | ||
| {"SyncLocalClockBeforeEpoch", ErrorCodes::SyncLocalClockBeforeEpoch}, |
There was a problem hiding this comment.
This was never added to the string_to_error_code array.
There was a problem hiding this comment.
shouldn't the new errors be added to this map? and also assign error categories?
There was a problem hiding this comment.
I am reusing existing ErrorCode values and the two added to this list were already assigned error categories (they were just left off this list - I just verified the list and the only other one missing is UnknownError this seems to be intentional).
| [[fallthrough]]; | ||
| case ProtocolError::transact_before_upload: | ||
| [[fallthrough]]; | ||
| case ProtocolError::edge_reboot: |
There was a problem hiding this comment.
This is very clearly not unreachable as we reach this point if the server sends us this error code. "Unreachable" specifically means that it's a bug in our code if things ever get to that point.
| {"SyncConnectFailed", ErrorCodes::SyncConnectFailed}, | ||
| {"SyncConnectTimeout", ErrorCodes::SyncConnectTimeout}, | ||
| {"SyncInvalidSchemaChange", ErrorCodes::SyncInvalidSchemaChange}, | ||
| {"SyncLocalClockBeforeEpoch", ErrorCodes::SyncLocalClockBeforeEpoch}, |
There was a problem hiding this comment.
what does this error even mean?
There was a problem hiding this comment.
It's an error I added. We use 2015-01-01 as epoch and count the milliseconds since then for the changesets' timestamp. This error is thrown when the local clock is set before that date.
| } realm_sync_errno_session_e; | ||
|
|
||
| // These errors are intended for the edge server and are an error if received by a sync client | ||
| typedef enum realm_sync_errno_edge_e { |
There was a problem hiding this comment.
I don't think we should expose errors for the edge server. The only way we could get these is via a bug. I think getting an UnkownError for edge server only errors would be totally acceptable.
There was a problem hiding this comment.
Removed the edge server and wire protocol errors
Pull Request Test Coverage Report for Build michael.wilkersonbarker_1333Details
💛 - Coveralls |
4 participants
What, How & Why?
While creating PR #7981, it was discovered that there were 3 protocol errors defined in baas that were not supported in the client. Added definitions for these 3 errors so they will not display "unknown error".
One of these errors (
ProtocolError::edge_reboot) is only intended for the edge server and an assertion will be thrown by the client when it tries to convert this error to a status object.☑️ ToDos
[ ] 🚦 Tests (or not relevant)[ ] C-API, if public C++ API changed[ ]bindgen/spec.yml, if public C++ API changed