Description
Operating System
macOS Sonoma 14.5
Environment (if applicable)
Chrome
Firebase SDK Version
10.14.1
Firebase SDK Product(s)
AppCheck
Project Tooling
React app with Webpack and jest
Detailed Problem Description
What I was trying to achieve
I expected getToken(appCheckInstance, { forceRefresh: true })
to return a fresh App Check token or throw an error if the request failed (e.g., due to network issues or misconfiguration). This would allow proper error handling in my application.
What actually happened
When an error occurs, getToken
does not return or throw it. Instead, it silently returns the cached token (if a valid one exists), making failures undetectable programmatically.
Unexpected behavior
- Errors are logged internally in the console but not surfaced to the caller.
- This prevents proper error handling, as failures cannot be detected in code.
- This causes the third-party caller to receive the cached token without realizing that it is not the fresh token they requested, but instead the cached one.
Code Reference:
In the getToken
function, when an error occurs and forceRefresh
is true
, if a valid cached token exists, the error is assigned to the internalError
property of the token result object. However, this internalError
is never surfaced to the caller, and the function silently returns the cached token instead of propagating the error. This appears to be an oversight in handling the forceRefresh
case, specifically in error scenarios:
Relevant logs / Console output (observed in DevTools Console)
No Network Scenario:
[2025-02-26T00:26:51.162Z] @firebase/app-check: FirebaseError: AppCheck: ReCAPTCHA error. (appCheck/recaptcha-error).
Despite this logged error, getToken still returns the last cached token instead of failing.
Invalid ReCAPTCHA Site Key Scenario
When testing an incorrect ReCAPTCHA site key, the following message appeared in the console:
@firebase/app-check: AppCheck: Requests throttled due to 403 error. Attempts allowed again after 01d:00m:00s (appCheck/throttled).
Again, the error is logged internally but not returned to the caller, preventing proper handling of any errors.
Expected behavior
Errors should be returned or thrown instead of being logged silently. If throwing the error is not the preferred approach, I would suggest returning the error alongside the token to the third-party caller. This would provide the app with the necessary information to properly detect and handle the failure, ensuring that it is not silently overlooked.
Steps and code to reproduce issue
- Initialize Firebase App Check.
- Call getToken(appCheckInstance, { forceRefresh: true }).
- Simulate any error getting token such as network issue or invalid App Check configuration.
- Observe that the function still returns a cached token instead of throwing an error.
Activity
google-oss-bot commentedon Mar 1, 2025
I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
hsubox76 commentedon Mar 6, 2025
The error should be thrown in the case of the 403. I tried:
The catch worked correctly and logged "error [the error message]" when I forced a 403 error by misspelling the projectId:
The recaptcha error (this one actually happens for me on the bad site key - did you get your examples reversed?) seems like it's going down the wrong code path. We added the
internalError
in the case that other products (Firestore, Auth, etc.) are trying to get the token by callinggetToken
internally - we don't want them to not get a possibly still-valid token just because there was an error trying to get a new one. That was done in this PR: #6617 - maybe we need to adjust the logic to ensure it still throws when a user callsgetToken
directly.I think this can probably be fixed just by having the public-facing
getToken()
also throw onresult.internalError
here:firebase-js-sdk/packages/app-check/src/api.ts
Line 210 in 51465ce
nermeen-mattar commentedon Mar 9, 2025
Thank you, @hsubox76, for your response and for explaining the reasoning behind implementing internalError; that makes the intent clearer.
Regarding being able to catch the error correctly when misspelling the projectId, could you please try this while a valid cached token exists? The issue specifically occurs in this scenario—I should have made that clearer in the reproduction steps (I've now updated them). In this case, the error is silently suppressed instead of being surfaced.
While the cached token may still be valid, the purpose of forceRefresh: true is to fetch a new token, so returning the cached token in case of failure misleads the caller.
Regarding the error examples, I’ve double-checked and confirmed that they aren't reversed. However, in my examples, I misspelled the
reCAPTCHA Enterprise site key
from the Firebase console side. On the other hand, misspelling it in our codebase resulted in the following error:FirebaseError: AppCheck: ReCAPTCHA error. (appCheck/recaptcha-error)
I agree with your suggested fix, and I’ve also opened a PR with an alternative approach. I’m happy to proceed with whichever direction you recommend as this issue is blocking us because our implementation requires a fresh token when forceRefresh: true is set, and without error propagation, we can't handle failures properly.
Looking forward to your thoughts! Thanks again.
hsubox76 commentedon Mar 13, 2025
We don't want to make this change in
internal-api.ts
because that code applies to both public calls ofgetToken()
and internal calls, where we want too keep using a valid token, and not throw. Internal calls can also set forceRefresh=true. I've made a PR #8842 to change the publicgetToken()
only, plus address some other considerations like distinguishing initial throttling errors (failed network response) from subsequent errors due to throttling (no network call made).nermeen-mattar commentedon Mar 20, 2025
@hsubox76 Thank you for addressing this promptly. Appreciated!