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

app-check: getToken with forceRefresh:true swallows errors and returns cached token #8822

Open
nermeen-mattar opened this issue Mar 1, 2025 · 1 comment · May be fixed by #8829
Open

app-check: getToken with forceRefresh:true swallows errors and returns cached token #8822

nermeen-mattar opened this issue Mar 1, 2025 · 1 comment · May be fixed by #8829

Comments

@nermeen-mattar
Copy link

nermeen-mattar commented Mar 1, 2025

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

  1. Initialize Firebase App Check.
  2. Call getToken(appCheckInstance, { forceRefresh: true }).
  3. Simulate any error getting token such as network issue or invalid App Check configuration.
  4. Observe that the function still returns a cached token instead of throwing an error.
@nermeen-mattar nermeen-mattar added new A new issue that hasn't be categoirzed as question, bug or feature request question labels Mar 1, 2025
@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

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.

3 participants