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

[hermes] Realm.open throws error when checking if Realm exists #5235

Closed
bimusiek opened this issue Jan 9, 2023 · 8 comments
Closed

[hermes] Realm.open throws error when checking if Realm exists #5235

bimusiek opened this issue Jan 9, 2023 · 8 comments
Assignees

Comments

@bimusiek
Copy link
Contributor

bimusiek commented Jan 9, 2023

How frequently does the bug occur?

Always

Description

When trying to open Realm, it throws error

Error: Exception in HostFunction: 'onDiscard' is required
    at exists (native)
    at open (http://192.168.0.58:8081/index.freeyourmusic.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=io.stampapp.Stamp:297173:50)

on this line (bundled file thus the screenshot):

The config which we open the Realm with:

  • JSON serialized:
{
  "schema": [],
  "sync": {
    "user": {},
    "partitionValue": "d7297249-7b94-405c-92f9-163effecc6ad",
    "existingRealmFileBehavior": {
      "type": "openImmediately"
    },
    "newRealmFileBehavior": {
      "type": "openImmediately"
    },
    "clientReset": {}
  }
}

I have cut out the schema part as it was too long.

I have tracked it to this line of code https://github.com/realm/realm-js/blob/master/src/js_sync.hpp#L1308
But I cannot find any documentation about it:
https://www.mongodb.com/docs/realm/sdk/node/examples/reset-a-client-realm/
There is neither mention of onDiscard nor onRecovery.

Stacktrace & log output

No response

Can you reproduce the bug?

No

Reproduction Steps

No response

Version

11.3.0

What services are you using?

Atlas Device Sync

Are you using encryption?

No

Platform OS and version(s)

iOS 16.0.3 / React Native 0.70.6

Build environment

Which debugger for React Native: Flipper (hermes)

Cocoapods version

No response

@bimusiek
Copy link
Contributor Author

bimusiek commented Jan 9, 2023

Looking at the js_sync.hpp code, it should go into realm::ClientResyncMode::DiscardLocal mode. Looking at JSON.stringify of the config, it did not pick the clientReset.mode and it was undefined. Thus it went into realm::ClientResyncMode::RecoverOrDiscard but why would it throw an error in default mode though? Shouldn't it by default work?

@takameyer
Copy link
Contributor

@bimusiek Thanks for reporting this. We will investigate.

@bimusiek
Copy link
Contributor Author

@takameyer Awesome, the fix for us was to change mode to Realm.ClientResetMode.DiscardUnsyncedChanges and adding

    if (!realmConfig.sync.clientReset) {
      // https://github.com/realm/realm-js/issues/5235
      throw new Error(`Missing client reset mode for Realm`);
    }

just to be sure it won't happen again.

@kneth
Copy link
Member

kneth commented Jan 18, 2023

@bimusiek

realm::ClientResyncMode::DiscardLocal is the same as Realm.ClientResetMode.DiscardUnsyncedChanges. In version 11.1.0 we changed the name to Realm.ClientResetMode.DiscardUnsyncedChanges to align with Realm SDKs but kept DiscardLocal in other to avoid breaking the API.

Default is currently realm::ClientResyncMode::RecoverOrDiscard (aligned with other Realm SDKs) and it requires two callbacks: onDiscard and onRecovery. We can add default implementations for these two callback - for example writing to the log.

@kneth
Copy link
Member

kneth commented Jan 18, 2023

I have opened #5288 but I don't think it solved it all.

@bimusiek
Copy link
Contributor Author

I will close the issue as the fix is working for us. Personaly I think that if there is default ClientResetMode that requires additional parameters, it should implement those by default.

It would be unexpected to use Realm and get native error saying that onDiscard is not implemented, hard to understand as there is no stacktrace pointing it to ClientResetMode configuration.

I only found the culprit because github found the error message in the native code and I figured it out.

@kneth
Copy link
Member

kneth commented Jan 24, 2023

@bimusiek Yes, I agree the default mode shouldn't require any configuration. To be honest, we haven't settled on what the default callback should do - maybe console.warn() something?

@bimusiek
Copy link
Contributor Author

As far as I understand the C++ code, the callbacks are simply called after Realm tried to recover or failed to recover and discarded the old one. Which means, why are those even required? I would either put default callbacks that do nothing or do not require those at all in the core. Console.warn is ok, but you will only see it when ClientReset happens, so hardly anyone will see it.

From dev perspective, I can assume that default behaviour is RecoverOrDiscard with no callbacks and if I want to be notified in the runtime that this happened, then I will put my own Realm.Configuration with proper callbacks.

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

No branches or pull requests

3 participants