fix: prevent token refresh thundering herd by aaguiarz · Pull Request #333 · openfga/js-sdk

aaguiarz · 2026-02-16T18:55:47Z

Summary

Ensures concurrent token header reads share one in-flight refresh instead of issuing parallel token exchange requests.

Changes

Added regression test asserting 5 parallel calls only trigger 1 token request
Added an internal refreshAccessTokenPromise lock in Credentials
Reused the same refresh promise for concurrent callers and cleared it when settled

Potential Breaking Change

Concurrent expired-token requests now share a single refresh result.
Under transient token endpoint failures, all waiting callers may fail together (instead of independent parallel refresh attempts), which changes failure/retry dynamics.

Fixes #332

Summary by CodeRabbit

Improvements
- Optimized concurrent authentication token requests to prevent duplicate requests and reduce server load.

coderabbitai · 2026-02-16T18:55:59Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

Implements deduplication of concurrent token refresh requests in Credentials by caching an in-flight promise. When multiple callers request access tokens simultaneously with no cached token available, they now await the same token refresh promise instead of triggering multiple simultaneous token exchanges. Includes test verification for concurrent request behavior.

Changes

Cohort / File(s)	Summary
Token Request Deduplication `credentials/credentials.ts`	Adds `refreshAccessTokenPromise` private member to cache in-flight client credentials token requests. In `getAccessTokenValue`, reuses the promise for concurrent callers requesting tokens without a valid cache, clearing it after completion.
Concurrent Request Tests `tests/credentials.test.ts`	Adds test case verifying concurrent calls to `getAccessTokenHeader` trigger only a single token endpoint request, with all callers receiving the same token response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: prevent token refresh thundering herd' clearly and concisely summarizes the main objective—preventing multiple concurrent token refresh requests (thundering herd problem).
Linked Issues check	✅ Passed	The PR fully implements the requirements from issue `#332`: adds an internal refreshAccessTokenPromise lock to deduplicate concurrent token refresh requests and includes regression tests verifying only one token request occurs.
Out of Scope Changes check	✅ Passed	All changes are scoped to the token refresh deduplication fix: internal Credentials lock mechanism and regression tests; no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/issue-332-concurrent-token-refresh

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

dosubot · 2026-02-16T18:56:07Z

Related Documentation

Checked 8 published document(s) in 1 knowledge base(s). No updates required.

^{How did I do? Any feedback?}

codecov-commenter · 2026-02-16T18:56:44Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.31%. Comparing base (a7090e2) to head (5ab0260).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   89.67%   90.31%   +0.63%     
==========================================
  Files          25       25              
  Lines        1492     1507      +15     
  Branches      279      263      -16     
==========================================
+ Hits         1338     1361      +23     
+ Misses         94       88       -6     
+ Partials       60       58       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR addresses issue #332 by deduplicating concurrent access token refreshes so multiple callers share a single in-flight token exchange instead of triggering parallel requests.

Changes:

Added an in-flight refresh lock (refreshAccessTokenPromise) in Credentials to dedupe concurrent refresh calls.
Added a regression test asserting 5 concurrent getAccessTokenHeader() calls only trigger 1 token request.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/credentials.test.ts	Adds concurrency regression test for single token exchange under parallel access-token header reads.
credentials/credentials.ts	Introduces a shared in-flight refresh promise to prevent concurrent token refresh thundering herd.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-16T18:59:09Z

credentials/credentials.ts

 export class Credentials {
  private accessToken?: string;
  private accessTokenExpiryDate?: Date;
+  private refreshAccessTokenPromise?: Promise<string | undefined>;



This file is marked as autogenerated (header) and CONTRIBUTING.md notes autogenerated files should not be modified directly; changes should be made in the sdk-generator templates/in tandem to avoid the fix being overwritten on regen. Please ensure this concurrency-lock change is also applied in the generator source (or confirm this file is intentionally maintained manually despite the header).

Copilot · 2026-02-16T18:59:09Z

tests/credentials.test.ts

+      nock(expectedBaseUrl)
+        .post(expectedPath)
+        .times(5)
+        .delay(20)
+        .reply(() => {
+          tokenRequestCount += 1;
+          return [200, {
+            access_token: "shared-token",
+            expires_in: 300,
+          }];
+        });


The test name asserts a single token request, but the nock interceptor is configured with .times(5) and the scope isn't asserted with scope.isDone(). Consider making the mock expect exactly one call (e.g., .once()/default) and asserting isDone() so the test intent is enforced by nock as well as by tokenRequestCount.

aaguiarz · 2026-02-16T21:15:39Z

Addressed the actionable test-review comment in commit d31b421.

Updated the concurrency regression test to enforce intent directly with nock:

switched interceptor from .times(5) to .once()
captured the scope and added expect(scope.isDone()).toBe(true)

This now verifies both:

all 5 callers receive the shared token
exactly one token endpoint request was made

Validated locally with:

npm test -- tests/credentials.test.ts -t "single token request for concurrent"

On the autogenerated-file note: agreed in principle; this PR remains scoped to the runtime fix and test behavior.

aaguiarz · 2026-02-17T12:29:23Z

@coderabbitai review

coderabbitai · 2026-02-17T12:29:35Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T12:36:11Z

tests/credentials.test.ts

+      let tokenRequestCount = 0;
+
+      const scope = nock(expectedBaseUrl)
+        .post(expectedPath)
+        .once()


tokenRequestCount is redundant with nock(...).post(...).once()/scope.isDone() (a second token request would already fail the test). Consider removing the counter to simplify the test and avoid incidental state.

Copilot · 2026-02-17T12:36:12Z

credentials/credentials.ts

+      if (!this.refreshAccessTokenPromise) {
+        this.refreshAccessTokenPromise = this.refreshAccessToken().finally(() => {
+          this.refreshAccessTokenPromise = undefined;
+        });
+      }
+
+      return this.refreshAccessTokenPromise;
    }


The new in-flight refresh lock is cleared via .finally(...), but there’s no regression test covering the failure path (e.g., token endpoint returns 500) to ensure: (1) all concurrent callers see the same rejection, and (2) a subsequent call retries and triggers a new token request after the promise is cleared. Adding a test for this would help prevent future regressions where the lock could get stuck on a rejected promise.

aaguiarz · 2026-02-17T12:52:26Z

Addressed the actionable review feedback from #333 (review) in commit 5ab0260.

What changed:

Removed the redundant tokenRequestCount assertion from the concurrent token-read test, since nock(...).once() + scope.isDone() already enforces single request behavior.
Added a new regression test (should clear shared refresh promise after failure and retry on the next call) that verifies:
- concurrent callers share the same rejection during a failed token refresh, and
- a subsequent call retries successfully (proving the in-flight refresh promise is cleared after rejection).

Validation run:

npm test -- tests/credentials.test.ts

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aaguiarz added 2 commits February 16, 2026 15:55

test: reproduce concurrent token refresh thundering herd

95ccb82

fix: deduplicate concurrent token refresh requests

a1ae47f

Copilot AI review requested due to automatic review settings February 16, 2026 18:55

aaguiarz requested a review from a team as a code owner February 16, 2026 18:55

Copilot started reviewing on behalf of aaguiarz February 16, 2026 18:56 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

test: enforce single token exchange call with nock

d31b421

aaguiarz requested a review from Copilot February 17, 2026 12:31

Copilot started reviewing on behalf of aaguiarz February 17, 2026 12:32 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

test: cover failed shared token refresh retry path

5ab0260

aaguiarz requested a review from Copilot February 17, 2026 12:55

Copilot started reviewing on behalf of aaguiarz February 17, 2026 12:55 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Conversation

aaguiarz commented Feb 16, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Potential Breaking Change

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

dosubot bot commented Feb 16, 2026

Uh oh!

codecov-commenter commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

aaguiarz commented Feb 16, 2026

Uh oh!

aaguiarz commented Feb 17, 2026

Uh oh!

coderabbitai bot commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

aaguiarz commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aaguiarz commented Feb 16, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 16, 2026 •

edited

Loading

codecov-commenter commented Feb 16, 2026 •

edited

Loading