firebase-js-sdk icon indicating copy to clipboard operation
firebase-js-sdk copied to clipboard

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

Open vthommeret opened this issue 1 year ago • 31 comments

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 avatar Dec 19 '23 07:12 vthommeret

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

vthommeret avatar Dec 19 '23 19:12 vthommeret

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

looptheloop88 avatar Dec 20 '23 12:12 looptheloop88

"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?

hsubox76 avatar Dec 20 '23 17:12 hsubox76

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 avatar Dec 22 '23 23:12 vthommeret

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 avatar Dec 26 '23 07:12 vthommeret

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);
}

vthommeret avatar Jan 04 '24 01:01 vthommeret

Any update on this? I have the same issue

liammaher avatar Jan 07 '24 16:01 liammaher

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

vthommeret avatar Jan 08 '24 23:01 vthommeret

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

hsubox76 avatar Jan 09 '24 18:01 hsubox76

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

lukeromanowicz avatar Jan 23 '24 17:01 lukeromanowicz

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

vthommeret avatar Jan 26 '24 21:01 vthommeret

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 avatar Jan 26 '24 22:01 hsubox76

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 avatar Jan 26 '24 22:01 hsubox76

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 avatar Jan 26 '24 22:01 hsubox76

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

hsubox76 avatar Jan 26 '24 23:01 hsubox76

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.

lukeromanowicz avatar Jan 29 '24 09:01 lukeromanowicz

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.

hsubox76 avatar Feb 02 '24 18:02 hsubox76

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

vthommeret avatar Feb 03 '24 18:02 vthommeret

image

Seeing this issue again this morning with latest release.

vthommeret avatar Feb 07 '24 21:02 vthommeret

@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 avatar Feb 08 '24 09:02 lukeromanowicz

@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.

vthommeret avatar Feb 08 '24 19:02 vthommeret

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

bo-rik avatar Feb 09 '24 15:02 bo-rik

@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.

lukeromanowicz avatar Feb 09 '24 15:02 lukeromanowicz

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

It is happening on Safari, intermittently.

I3uckwheat avatar Feb 16 '24 17:02 I3uckwheat

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

jtovar2 avatar Feb 26 '24 00:02 jtovar2

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).

Paso avatar Feb 27 '24 10:02 Paso

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 avatar Mar 28 '24 16:03 lifter035

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

lifter035 avatar Apr 05 '24 14:04 lifter035

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.

Takur0 avatar Apr 13 '24 16:04 Takur0

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')

georgecartridge avatar May 16 '24 11:05 georgecartridge