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

SyncSession::wait_for_upload_completion does not return error if session is invalid #5365

Closed
cmelchior opened this issue Mar 30, 2022 · 10 comments
Assignees

Comments

@cmelchior
Copy link
Contributor

While trying to write some unit tests for Kotlin around waiting for upload and download I discovered something that looks like a bug.

  1. Open a synchronized Realm
  2. Restart Sync on the server using the Admin API, this correctly triggers a session error handler.
  3. Attempt to call SyncSession: wait_for_upload_completion and SyncSession::wait_for_download_completion

Expected result: I would expect the function to return some kind of error_code in the callback we provide.

Actual result: It returns succesfully.

@jbreams
Copy link
Contributor

jbreams commented May 20, 2022

Does the callback return successfully or does just calling wait_for_download_completion return successfully and the callback never gets called? If the latter, I think this works as designed. Or at least I think this works as its always worked.

@jbreams jbreams self-assigned this May 20, 2022
@nicola-cab nicola-cab removed their assignment May 20, 2022
@nielsenko
Copy link
Contributor

nielsenko commented May 31, 2022

The callback never gets called. Problem is we can have users hang forever on fx. await waitForUpload(), if a fatal sync error happens, such as a user writing to a realm with no subscriptions.

It gets logged, and an error is send back via callback/handler passed to realm_sync_client_set_error_handler, but the callback behind await waitForUpload is not called with an error, causing that particular call to hang.

This is less than stellar.

In dart a minimal sample would look something like:

realm.write(() {
  realm.add(ARealmObject());
});
await realm.syncSession.waitForUpload();

where realm is opened with flexible sync, but subscriptions has not been set up yet.

@nielsenko
Copy link
Contributor

Sorry, if I might be on the wrong issue here.

@cmelchior
Copy link
Contributor Author

No, that is exactly the use case for Kotlin as well.

@nicola-cab
Copy link
Member

It seems to me that it is either this:

or this:

That is causing the problem, we should probably just invoke the callback with some error code if no subscription is present.

@cmelchior
Copy link
Contributor Author

Note, it is for all errors, not just subscriptions, i.e. this also happens if the server sends a Client Reset

@sync-by-unito
Copy link

sync-by-unito bot commented Sep 6, 2022

➤ Jonathan Reams commented:

I think this basically works-as-designed or is at least a forever bug/design flaw. Unless the user has been logged out, sync sessions are never "invalid" they are always active, or about to become active, and so if you are able to access a SyncSession to request download completion notifications, then it is not invalid and requesting a notification is an okay thing to do, even if that notification is going to be processed in the distant future. Maybe there's a bug specifically in how client resets work that is causing us to not fire download completions after the client reset is complete? In core version 12.1.0 we made it so doing a write on a table that does not have a subscription will throw an error before you commit, so the specific FLX case called out here should be mitigated by that.

@nielsenko
Copy link
Contributor

@jbreams I agree. This particular case is no longer a problem.

@sync-by-unito
Copy link

sync-by-unito bot commented Sep 26, 2022

➤ Jonathan Reams commented:

[~christian.melchior@mongodb.com] is there more to do here? Is this a duplicate of RCORE-1001?

@sync-by-unito sync-by-unito bot closed this as completed Oct 26, 2022
@sync-by-unito
Copy link

sync-by-unito bot commented Oct 26, 2022

➤ Jonathan Reams commented:

It's been over a month since I asked whether this was a duplicate of RCORE-1001 and I never got a response, so I'm going to close this as a duplicate. If there's more work to do here that isn't covered by RCORE-1001, please reach out and re-open.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants