firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.82k stars 885 forks source link

two attempts to connect to auth emulator cause config error #6824

Open joaomelo opened 1 year ago

joaomelo commented 1 year ago

[REQUIRED] Environment info

firebase-tools: 11.14.3 Platform: Windows 11

[REQUIRED] Test case

This repo commit.

[REQUIRED] Steps to reproduce

Clone the repo, install the project dependencies with npm i and firebase tools globally, open the windows terminal, and start emulators with firebase emulators:exec --project demo-test "vitest run"

[REQUIRED] Expected behavior

All test cases are able to connect to firebase emulators.

[REQUIRED] Actual behavior

After a test case connect to the emulators, the following ones will throw an error of config failed. See image:

error

But if we changed the connection code from this...

function connectToFirebase(connection) {
  const { useEmulators, ...config } = connection;

  const app = initializeApp(config);
  const auth = getAuth(app);
  const db = getFirestore(app);

  if (useEmulators) {
    connectAuthEmulator(auth, "http://localhost:9099", {
      disableWarnings: true,
    });
    connectFirestoreEmulator(db, "localhost", 8080);
  }

  const firebase = { app, auth, db };
  return firebase;
}

to this...

function connectToFirebase(connection) {
  const { useEmulators, ...config } = connection;

  const app = initializeApp(config);
  const auth = getAuth(app);
  const db = getFirestore(app);

  if (useEmulators && !auth.config.emulator) {
    connectAuthEmulator(auth, "http://localhost:9099", {
      disableWarnings: true,
    });
    connectFirestoreEmulator(db, "localhost", 8080);
  }

  const firebase = { app, auth, db };
  return firebase;
}

all connections go through all right.

pass

I'm claiming this is a bug since there is no documented spec that claims a second connection attempt would fail and that the hack adopted here (!auth.config.emulator) uses an undocumented property that could change anytime. The consequence would be a fragile code that can be easily broken in a future path or minor release.

Thanks. JM

christhompsongoogle commented 1 year ago

Thanks for the feedback - I'll forward to the team.

google-oss-bot commented 1 year ago

I found a few problems with this issue:

prameshj commented 1 year ago

Thanks for filing this. Is it possible that network calls are being made before a second call to auth emulator? Any network call will result in the init flag set to false - https://github.com/firebase/firebase-js-sdk/blob/04dcdbb0f5b19e51082a846463d84aadb77e98d4/packages/auth/src/api/index.ts#L135, so future connectAuthEmulator will fail.

we can update the docs to clarify this.

joaomelo commented 1 year ago

Hi @prameshj, Thanks for answering.

I don't think a network call is made to the auth service before connecting to the auth emulator. The firebase setup is made at the beginning of the program lifecycle. This is the code:

function connectToFirebase(connection) {
  const { useEmulators, ...config } = connection;

  const app = initializeApp(config);
  const auth = getAuth(app);
  const db = getFirestore(app);

  if (useEmulators) {
    connectAuthEmulator(auth, "http://localhost:9099", {
      disableWarnings: true,
    });
    connectFirestoreEmulator(db, "localhost", 8080);
  }

  const firebase = { app, auth, db };
  return firebase;
}

But there is always the possibility of a bug in my code. Any suggestions on how I could ensure that and get back to you? Like two methods to spy on the execution order.

prameshj commented 1 year ago

Is "connectToFirebase" invoked before each test case? I wonder if this is because each test case makes some network calls (by calling some auth APIs) and hence the issue.

We could consider exposing "emulatorInitialized()" that returns a boolean, to be checked before invoking connectAuthEmulator. WDYT, @yuchenshi @sam-gc

joaomelo commented 1 year ago

Is "connectToFirebase" invoked before each test case? Yes, it is.

bdotsamir commented 1 year ago

Hi there! Thought I would chip in my two cents here,

I'm using react, and I made the rookie mistake of calling connectAuthEmulator() outside of a useEffect hook with an empty array of dependencies (thus acting like componentDidMount). Changing this made the error disappear.

Hope this helps!

(p.s: your repo link is broken)