Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RKOTLIN-1096] Map is_fatal to UnrecoverableSyncException #1768

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

clementetb
Copy link
Contributor

closes #1767

@cla-bot cla-bot bot added the cla: yes label May 29, 2024
@clementetb clementetb self-assigned this May 29, 2024

ErrorCode.RLM_ERR_SYNC_PROTOCOL_INVARIANT_FAILED,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these 3 all now under Fatal UnrecoverableSyncException?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least RLM_ERR_SYNC_PERMISSION_DENIED isn't, but shouldn't now any fatal (unrecoverable) errors haveis_fatal set to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with core, is_fatal is a better property to identify unrecoverable errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems safest to keep this, unless you are absolutely certain that core throws the three error codes with is_fatal set. If not we will suddenly throw them as SyncExceptions instead.

channel.send(AssertionError("Realm was successfully opened"))
}
}

val error = channel.receiveOrFail()
assertTrue(error is UnrecoverableSyncException, "Was $error")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a test that checks UnrecoverableSyncException?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the only test that mentioned UnrecoverableSyncException. I am going to add some tests using realm_sync_session_handle_error_for_testing

@clementetb clementetb changed the base branch from main to releases May 30, 2024 13:44
@clementetb clementetb force-pushed the ct/use_is_fatal branch 2 times, most recently from 2f020e6 to 29c347b Compare June 3, 2024 21:06
CHANGELOG.md Outdated Show resolved Hide resolved
@clementetb clementetb merged commit 422d9cb into releases Jun 4, 2024
55 checks passed
@clementetb clementetb deleted the ct/use_is_fatal branch June 4, 2024 15:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal sync exceptions are not thrown as UnrecoverableSyncException
3 participants