Skip to content

fix: prevent token refresh thundering herd #333

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.

Sign up for GitHub

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

Open
aaguiarz wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: prevent token refresh thundering herd #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95ccb82
test: reproduce concurrent token refresh thundering herd
aaguiarz Feb 16, 2026
a1ae47f
fix: deduplicate concurrent token refresh requests
aaguiarz Feb 16, 2026
d31b421
test: enforce single token exchange call with nock
aaguiarz Feb 16, 2026
5ab0260
test: cover failed shared token refresh retry path
aaguiarz Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion credentials/credentials.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const DEFAULT_TOKEN_ENDPOINT_PATH = "oauth/token";
export class Credentials {
private accessToken?: string;
private accessTokenExpiryDate?: Date;
private refreshAccessTokenPromise?: Promise<string | undefined>;

Comment on lines 45 to 49
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
public static init(configuration: { credentials: AuthCredentialsConfig, telemetry: TelemetryConfiguration, baseOptions?: any }, axios: AxiosInstance = globalAxios): Credentials {
return new Credentials(configuration.credentials, axios, configuration.telemetry, configuration.baseOptions);
Expand Down Expand Up @@ -141,7 +142,13 @@ export class Credentials {
return this.accessToken;
}

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

return this.refreshAccessTokenPromise;
}
Comment on lines +145 to 152
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}

Expand Down
88 changes: 88 additions & 0 deletions tests/credentials.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,5 +537,93 @@ describe("Credentials", () => {

expect(scope.isDone()).toBe(true);
});

test("should send a single token request for concurrent access token reads", async () => {
const apiTokenIssuer = "issuer.fga.example";
const expectedBaseUrl = "https://issuer.fga.example";
const expectedPath = `/${DEFAULT_TOKEN_ENDPOINT_PATH}`;

const scope = nock(expectedBaseUrl)
.post(expectedPath)
.once()
.delay(20)
.reply(200, {
access_token: "shared-token",
expires_in: 300,
});

const credentials = new Credentials(
{
method: CredentialsMethod.ClientCredentials,
config: {
apiTokenIssuer,
apiAudience: OPENFGA_API_AUDIENCE,
clientId: OPENFGA_CLIENT_ID,
clientSecret: OPENFGA_CLIENT_SECRET,
},
} as AuthCredentialsConfig,
undefined,
mockTelemetryConfig,
);

const headers = await Promise.all(
Array.from({ length: 5 }, () => credentials.getAccessTokenHeader())
);

headers.forEach(header => {
expect(header?.value).toBe("Bearer shared-token");
});
expect(scope.isDone()).toBe(true);
});

test("should clear shared refresh promise after failure and retry on the next call", async () => {
const apiTokenIssuer = "issuer.fga.example";
const expectedBaseUrl = "https://issuer.fga.example";
const expectedPath = `/${DEFAULT_TOKEN_ENDPOINT_PATH}`;

const scope = nock(expectedBaseUrl)
.post(expectedPath)
.once()
.reply(404, {
code: "not_found",
message: "token exchange failed",
})
.post(expectedPath)
.once()
.reply(200, {
access_token: "recovered-token",
expires_in: 300,
});

const credentials = new Credentials(
{
method: CredentialsMethod.ClientCredentials,
config: {
apiTokenIssuer,
apiAudience: OPENFGA_API_AUDIENCE,
clientId: OPENFGA_CLIENT_ID,
clientSecret: OPENFGA_CLIENT_SECRET,
},
} as AuthCredentialsConfig,
undefined,
mockTelemetryConfig,
);

const results = await Promise.allSettled(
Array.from({ length: 5 }, () => credentials.getAccessTokenHeader())
);
const rejected = results.filter((result): result is PromiseRejectedResult => result.status === "rejected");

expect(rejected).toHaveLength(5);
expect(rejected[0].reason).toBe(rejected[1].reason);
expect(rejected[1].reason).toBe(rejected[2].reason);
expect(rejected[2].reason).toBe(rejected[3].reason);
expect(rejected[3].reason).toBe(rejected[4].reason);

const header = await credentials.getAccessTokenHeader();

expect(header?.value).toBe("Bearer recovered-token");
expect(scope.isDone()).toBe(true);
});
});
});
Loading