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-1102] Remove nullability check in SubscriptionSetImpl.waitForSynchronization #1778

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

clementetb
Copy link
Contributor

@clementetb clementetb commented Jun 7, 2024

closes #1777

Removes a nullability check on the synchronization error message that is causing a null pointer exception crash

Copy link
Contributor

@rorbech rorbech left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -369,7 +369,7 @@ class CredentialsTests {
payload = mapOf("mail" to TestHelper.randomEmail(), "id" to 0)
)

assertFailsWithMessage<AuthException>("unauthorized") {
assertFailsWithMessage<AuthException>("Authentication failed") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. Weird that this has to go the other way. Was updated on the server two weeks ago and seemed pretty consistent with unauthorized throughout the server 🤔 But I guess it is ok, since it is just a test assertion 🤷

@@ -127,7 +127,7 @@ internal class SubscriptionSetImpl<T : BaseRealm>(
if (result) {
return true
} else {
throw BadFlexibleSyncQueryException(errorMessage!!)
throw BadFlexibleSyncQueryException(errorMessage)
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this nullable because RealmInterop.realm_sync_subscriptionset_error_str can return null/empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it looks like it.

@clementetb clementetb merged commit c8e3780 into releases Jun 7, 2024
55 checks passed
@clementetb clementetb deleted the ct/fix-null-pointer-subscriptionsetimpl branch June 7, 2024 13:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 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.

3 participants