firebase / firebase-js-sdk

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

Auth not working properly in Safari: AbortError in indexeddb.ts #7888

Open vthommeret opened 5 months ago

vthommeret commented 5 months ago

Operating System

macOS 14.2

Browser Version

Safari 17.2

Firebase SDK Version

10.7.1

Firebase SDK Product:

Auth

Describe your project's tooling

Next.JS 14.0.4

Describe the problem

I'm running into an issue where neither authStateReady nor onAuthStateChanged is working consistently in Safari (i.e. 90% of the time authStateReady never resolves or onAuthStateChanged isn't called). It's a challenging bug because:

  1. It only seems to happen in production
  2. It doesn't occur in private browsing
  3. It doesn't occur in Chrome
  4. Sometimes I get an AbortError which calls out specific IndexedDB issues.
  5. Sometimes I don't, but it still doesn't work.

I was able to track down when one of the errors occurred:

"Firebase: Error thrown when reading from IndexedDB. Original error: Error looking up record in object store by key range. (app/idb-get). Unhandled Promise Rejection: AbortError: AbortError

I set a breakpoint and it appears that the error is coming from line 68 in indexeddb.ts:

https://github.com/firebase/firebase-js-sdk/blob/f854abe5b9be5fa2edf0df9bea971e1cbf9a3746/packages/app/src/indexeddb.ts#L68C29-L68C29

One thing I've noticed is that in tabs where the page isn't working, if I look at IndexedDB there's no data in firebase-heartbeat-storage or firebaseLocalStorageDb but on the pages where it is working, there is data.

I have a theory that there's a bug / some interaction with Safari's IndexedDB support, but I'm not sure what exactly. As you can imagine this bug is somewhat a showstopper since users can't log in :-)

Steps and code to reproduce issue

This is what I have for a minimal repro:

useEffect(() => {
  // Initialize Firebase
  initializeApp(firebaseConfig)

  const auth = getAuth();

  (async () => {
    console.log('calling authStateReady')
    try {
      await auth.authStateReady()
    } catch (error) {
      console.log('Unable to call authStateReady', error)
    }
    console.log('called authStateReady')
    setUser(auth.currentUser)
  })()

}, [])

When auth works, called authStateReady is called, otherwise not. I have a live demo here: https://repro.easiestcourses.com/minimal

If it says User: pending then the bug is present. If it switched to User: signed out then it's working as expected. As I mentioned earlier, one challenging thing is that this doesn't happen consistently so that page may or may not work in Safari if you load it.

vthommeret commented 5 months ago

I was looking through some old issues and it seems very similar to https://github.com/firebase/firebase-js-sdk/issues/6871 and https://github.com/firebase/firebase-js-sdk/pull/7272

Is it possible that a non-critical error for platform logging is is causing authentication to fail? cc @hsubox76 @dwyfrequency

looptheloop88 commented 5 months ago

Thanks for bringing this issue to our attention @vthommeret. Let me check with the team if this issue is also related to #7829.

hsubox76 commented 5 months ago

"Firebase: Error thrown when reading from IndexedDB. Original error: Error looking up record in object store by key range. (app/idb-get). Unhandled Promise Rejection: AbortError: AbortError

Is this a warning or an error? Are they both in the same log or are these two separate logs? I'm asking because the line of code you're referring to should result in a console.warn as it's wrapped in a try/catch that does that, and the message should look like the first line (see line 81 in your link). Is the second line part of that warning string or is it a separate thrown error?

vthommeret commented 5 months ago

Hi @hsubox76, thanks for the reply! They are two separate logs. I'm no longer seeing the error so it's possible it was a separate issue. I'm going to close this issue for now and will reopen if it reoccurs and I can get additional detail. Thanks!

vthommeret commented 5 months ago

So this now re-appearing a few days later. Something I'm seeing is it may spin for up to a minute and occasionally it will finally authenticate.

I'm also now seeing this when trying to sign in:

FirebaseError: Firebase: Error (auth/network-request-failed)

I haven't touched anything around sign in or authentication so I don't think this error is in my code. Same behavior as before — loading in Chrome or a private Safari window exhibits no issues. I've also disabled all my extensions which doesn't seem to have fixed anything.

One interesting thing is quitting and reopening Safari seems to temporarily fix it. I'm not seeing the the AbortErrors or IndexedDB errors — it just seems like Firebase Auth is hanging / not returning auth status due to some state in Safari.

vthommeret commented 5 months ago

I'm able to track down the AbortError to this line:

reject(tx.error || new DOMException('AbortError', 'AbortError')); which is in wrap-idb-value.js which I think is a dependency of firebase-js-sdk but unfortunately I can't seem to get an actual stack trace. Full referenced code is below.

One thing I discovered is if I refresh the page multiple times in fast succession it will eventually succeed (but still reoccur later).

image
function cacheDonePromiseForTransaction(tx) {
    // Early bail if we've already created a done promise for this transaction.
    if (transactionDoneMap.has(tx))
        return;
    const done = new Promise((resolve, reject) => {
        const unlisten = () => {
            tx.removeEventListener('complete', complete);
            tx.removeEventListener('error', error);
            tx.removeEventListener('abort', error);
        };
        const complete = () => {
            resolve();
            unlisten();
        };
        const error = () => {
            reject(tx.error || new DOMException('AbortError', 'AbortError'));
            unlisten();
        };
        tx.addEventListener('complete', complete);
        tx.addEventListener('error', error);
        tx.addEventListener('abort', error);
    });
    // Cache it for later retrieval.
    transactionDoneMap.set(tx, done);
}
liammaher commented 4 months ago

Any update on this? I have the same issue

vthommeret commented 4 months ago

OK I'm finally able to get a stack trace for the AbortError. It's a little hard to track down with all the anonymous functions but the Firebase call is here (the "transaction()" call):

https://github.com/firebase/firebase-js-sdk/blob/d7ace80d44ec870c3117cfed04ae6a1988c03c8e/packages/app/src/indexeddb.ts#L73

async function readHeartbeatsFromIndexedDB(app) {
    try {
        const db = await getDbPromise();
        const result = await db
            .transaction(STORE_NAME) // Triggers AbortError
            .objectStore(STORE_NAME)
            .get(computeKey(app));
        return result;
    }
    catch (e) {
        if (e instanceof FirebaseError) {
            logger.warn(e.message);
        }
        else {
            const idbGetError = ERROR_FACTORY.create("idb-get" /* AppError.IDB_GET */, {
                originalErrorMessage: e === null || e === void 0 ? void 0 : e.message
            });
            logger.warn(idbGetError.message);
        }
    }
}

It seems like this is very similar to the earlier uncaught heartbeat errors #6871 and #7272. That said it looks like errors are caught in that code so I'm not sure why this is still bubbling up and causing errors.

The Firebase "transaction()" referenced above is line 625 in index.esm below.

stack-trace
hsubox76 commented 4 months ago

This may be fixed by https://github.com/firebase/firebase-js-sdk/pull/7890/ - should make it into the next release.

lukeromanowicz commented 4 months ago

I've recently faced the same issue on 10.5.2 and the 10.7.2 release does not seem to fix it

Edit: apparently Safari doesn't like [ ] characters in [default]!undefined key that is generated when app name is not provided in initializeApp

vthommeret commented 4 months ago

Yes can confirm still seeing this issue with 10.7.2. I'm seeing it now locally as well. Right now the only "fix" I have is refreshing the page multiple times in fast succession since it seems to be some type of race condition.

image
hsubox76 commented 4 months ago

I've recently faced the same issue on 10.5.2 and the 10.7.2 release does not seem to fix it

Edit: apparently Safari doesn't like [ ] characters in [default]!undefined key that is generated when app name is not provided in initializeApp

If this is indeed the problem, we should be able to fix it quickly.

Edit: Wait, I have more questions.

(1) The first part of that generated key is app.name, which is [DEFAULT] as expected, but the second part after the ! should be appId. Why is appId undefined in your app? Did you provide a full firebaseConfig object to initializeApp?

(2) Does Safari have a problem with the ! character as well? Apparently the spec says the key must be a valid JS identifier, which would also exclude the !.

hsubox76 commented 4 months ago

I'm testing keys with odd characters in Safari 17.3 and it doesn't seem to have a problem:

Screenshot 2024-01-26 at 2 43 06 PM
hsubox76 commented 4 months ago

Thanks to that stack trace though, it looks like the unhandled exception is being thrown at this point in the idb code (it says line 77 but I think it's here, maybe because we don't have the latest version of idb):

https://github.com/jakearchibald/idb/blob/eb2fc14bb3588d09aaa5e86a83bf3519b06e10b3/src/wrap-idb-value.ts#L86

That leads me to think that the problem in our code might be here:

https://github.com/firebase/firebase-js-sdk/blob/master/packages/app/src/indexeddb.ts#L76

We await the result of get() but we don't await tx.done, which writeHeartbeatsToIndexedDB does below. I think the error is happening in tx.done and since we don''t await it in the try/catch block, it's not caught.

Unfortunately, since this is hard to reproduce I can implement this "fix" but it will just be a shot in the dark and we won't know if it's related to the issue until someone who is having this issue tries it. I'll try to fix it and get it into next week's release.

hsubox76 commented 4 months ago

Here's the possible fix: https://github.com/firebase/firebase-js-sdk/pull/7984

lukeromanowicz commented 4 months ago

I've recently faced the same issue on 10.5.2 and the 10.7.2 release does not seem to fix it Edit: apparently Safari doesn't like [ ] characters in [default]!undefined key that is generated when app name is not provided in initializeApp

If this is indeed the problem, we should be able to fix it quickly.

Edit: Wait, I have more questions.

(1) The first part of that generated key is app.name, which is [DEFAULT] as expected, but the second part after the ! should be appId. Why is appId undefined in your app? Did you provide a full firebaseConfig object to initializeApp?

(2) Does Safari have a problem with the ! character as well? Apparently the spec says the key must be a valid JS identifier, which would also exclude the !.

1) Auth in my application was working perfectly well on all browsers except Safari with both name and appId being not defined in the config. Setting the app name alone to get rid of [default] made the error go away. 2) Since I used "app" as a name, the key I currently see is app!undefined and the error is gone.

Perhaps there was some unique scenario that was causing Safari to interpret the key differently than it usually does.

hsubox76 commented 4 months ago

Just released 10.8.0 yesterday which contains the possible fix (see above). As mentioned above, since we don't have a way to reproduce this, the fix is just a guess, so try it and see if it does any good.

vthommeret commented 3 months ago

Thanks @hsubox76! I updated and will track to see if this comes up again.

vthommeret commented 3 months ago
image

Seeing this issue again this morning with latest release.

lukeromanowicz commented 3 months ago

@vthommeret do you have your app name set? e.g. firebaseApp = initializeFirebaseApp(firebaseConfig, 'MyApp'); If you do, it's important to refer to that app by getAuth(firebaseApp) every time you would normally use getAuth()

vthommeret commented 3 months ago

@vthommeret do you have your app name set? e.g. firebaseApp = initializeFirebaseApp(firebaseConfig, 'MyApp'); If you do, it's important to refer to that app by getAuth(firebaseApp) every time you would normally use getAuth()

@lukeromanowicz Can you clarify what error you're seeing? I.e. The abort error I shared? The primary key for firebase-heartbeat-store is [DEFAULT] (i.e. I'm not setting an app name) followed by ! and my app ID (i.e. not undefined).

Auth mostly works maybe half he time but I'm not sure how this is related.

bo-rik commented 3 months ago

We can also confirm the issue still persists in version 10.8.0. When we wait for the Auth to become ready using the Auth.authStateReady()[1], it is never called (nor the success callback, neither the error callback). It just 'hangs'. It only happens on Safari on iOS (not on macOS). We observe the issue on iOS 17.3 on an iPhone 14 Pro. It does not happen consistently, rather about 1 out of 4 times. Refreshing the page does not fix the issue. Going into Private Mode and visiting the same URL unfreezes it, as well as force closing Safari and opening it again.

[1] https://firebase.google.com/docs/reference/js/auth.auth.md#authauthstateready

lukeromanowicz commented 3 months ago

@vthommeret I was randomly seeing the exact same error in Safari as you did and had [DEFAULT] as part of the key. The chunk !undefined that follows it was not relevant. After I declared the app name as "MyApp" which changed the key prefix in my case from [DEFAULT]... to MyApp..., the error was gone.

I3uckwheat commented 3 months ago

We are also experiencing this issue, is there a recommended version that does not experience this issue?

It is happening on Safari, intermittently.

jtovar2 commented 3 months ago

Hey faced the same issue with safari. Adding the application name to the initializeApp() function, and emptying the caches in safari developers tools fixed the issue

Paso commented 3 months ago

This is a common error among our users, on both OSX and iOS Safari. I have not been able to reproducs it myself but get the errors reported via Sentry, so I can see that is affects many users. The exact error I'm seeing is:

@firebase/app: Firebase: Error thrown when reading from IndexedDB. Original error: Error looking up record in object store by key range. (app/idb-get).
lifter035 commented 2 months ago

Just wanted to add that this has nothing to do with firebase. In our project we use multiple indexeddb to store some info and it seems to be that some condition triggers wipe of db data. We haven't figured out what it is yet but all the dbs are still there, their tables are still there, but there's nothing in the tables. Clearing cache fully in Safari and reloading sometimes helps. Potentially it might be a bug in webkit ITP https://webkit.org/tracking-prevention/#intelligent-tracking-prevention-itp but can't say for sure :) We tried enabling debug on that but it did not log anything on successful repro

lifter035 commented 1 month ago

Just another update. Not sure if it's relevant in this case but in our case this was only happening if you have dev tools enabled in Safari settings and after an hour, Safari in that case seems to wipe everything

Takur0 commented 1 month ago

Hey faced the same issue with safari. Adding the application name to the initializeApp() function, and emptying the caches in safari developers tools fixed the issue

Thanks @jtovar2!!

I fixed the issue using the above method too.

georgecartridge commented 2 weeks ago

Had the same issues on Safari, no problems with Chrome.

The solution from @jtovar2 fixed things - just by adding a name to the initializeApp() function like so:

// before
initializeApp(config)

// after
initializeApp(config, 'anyNameHere')