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

Database SDK should warn when it builds databaseUrl from projectId #2337

Open
samtstern opened this issue Jan 18, 2021 · 6 comments
Open

Database SDK should warn when it builds databaseUrl from projectId #2337

samtstern opened this issue Jan 18, 2021 · 6 comments

Comments

@samtstern
Copy link
Contributor

[READ] Step 1: Are you in the right place?

Yep

[REQUIRED] Step 2: Describe your environment

  • Android Studio version: all
  • Firebase Component: Database
  • Component version: 19.6.0

[REQUIRED] Step 3: Describe the problem

Steps to reproduce:

  1. Create a new project
  2. Add an Android app
  3. Download google-services.json
  4. Create the default RTDB instance on the project, choose Europe location

Because (3) precedes (4) the json file will not contain the firebase_url key and therefore the RTDB SDK will try and make up the database URL:

  public static FirebaseDatabase getInstance(@NonNull FirebaseApp app) {
    String databaseUrl = app.getOptions().getDatabaseUrl();
    if (databaseUrl == null) {
      if (app.getOptions().getProjectId() == null) {
        throw new DatabaseException(
            "Failed to get FirebaseDatabase instance: Can't determine Firebase Database URL. "
                + "Be sure to include a Project ID in your configuration.");
      }
      databaseUrl = "https://" + app.getOptions().getProjectId() + "-default-rtdb.firebaseio.com";
    }
    return getInstance(app, databaseUrl);
  }

However it doesn't print any warning when it does this, and for anyone who chooses Europe this is the wrong URL. See developer pain here:
firebase/codelab-friendlychat-android#137

Relevant Code:

See above

@schmidt-sebastian
Copy link
Contributor

@jfdt - Do you have thoughts here?

@jsdt
Copy link
Contributor

jsdt commented Jan 23, 2021

We can improve the error message to make it clear that the database is in a different region (and needs a different URL). I don't think we need to have a warning if the url is inferred and works correctly though.

@fredzqm
Copy link
Contributor

fredzqm commented Jan 23, 2021

Just to be clear even if the google-services.json is redownloaded after the database is created, it won't contain databaseUrl. Any non-us-central default databases must be connected with its full url passed into getInstance().

Thanks for bringing this up. We should work on improving the public documentation and surface a better error message in the SDK.

@fredzqm
Copy link
Contributor

fredzqm commented Jan 23, 2021

Sorry, I take it back. Creating the databaseUrl today will change the config.
I was confused because the decision settled a while ago was to discourage config redownloads.

@samtstern
Copy link
Contributor Author

FYI there are a lot of conversations going on about this topic, the future of the JSON plugin, etc. So before making any changes here make sure to talk to Mike D at least.

@jsdt that would imply that the warning only happens after the first failed operation, right? Or does the RTDB SDK eagerly try to connect to the backend when instantiated?

@schmidt-sebastian
Copy link
Contributor

The Android SDK only connects to the backend when there is work to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants