feat(js-sdk): support retry-after header by ryanpq · Pull Request #504 · openfga/sdk-generator

ryanpq · 2025-03-04T19:05:01Z

Description

Honor the retry-after header so that a client does not overwhelm the FGA server with requests.

References

Closes: openfga/js-sdk#208

Review Checklist

I have clicked on "allow edits by maintainers".
I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
The correct base branch is being used, if not main
I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

New Features
- Added configurable HTTP retries with exponential backoff and support for Retry-After headers.
- Improved resilience to network errors and refined status-based error handling for rate limits and server errors.
Bug Fixes
- More accurate retry delays and error mapping, reducing spurious failures and improving reliability.
Tests
- Expanded coverage for retry scenarios (including Retry-After and backoff), token exchange flows, and error handling.
- Reorganized test suites and added support for custom response headers in token exchange mocks.

rhamzeh · 2025-03-16T13:08:37Z

          fgaApi.check(baseConfig.storeId!, { tuple_key: tupleKey }, { retryParams: { maxRetry: 0 }})
        ).rejects.toThrow(FgaApiInternalError);
      });
+      it("should throw FgaApiInternalError for 500 error without Retry-After header", async () => {


Without the retry header, it should fall back to exponential backoff

rhamzeh · 2025-03-16T13:10:33Z

+        if (status === 429) {
+          retryDelayMs = randomTime(iterationCount, config.minWaitInMs);
+        } else if (status >= 500 && status !== 501) {
+          // For 5xx (except 501), we only retry if Retry-After was present


Let's change this so that we fall back to exponential backoff here too

rhamzeh · 2025-03-16T13:13:18Z

+  const secondsValue = parseInt(retryAfterValue, 10);
+  if (!isNaN(secondsValue)) {
+    const msValue = secondsValue * 1000;
+    if (msValue >= 1000 && msValue <= 1800000) {


rather than these being hard-coded, now that the go-sdk changes have been merged, you can use the following variables:

{{maxBackoffTimeInSec}}

Suggested change

if (msValue >= 1000 && msValue <= 1800000) {

if (msValue > 0 && msValue <= {{retryHeaderMaxAllowableDurationInSec}}) {

}

rhamzeh · 2025-03-16T13:14:14Z

+    const now = new Date();
+    const delayMs = dateValue.getTime() - now.getTime();
+
+    if (delayMs >= 1000 && delayMs <= 1800000) {


This logic looks repeated, can you extract it to it's own function and reference it above and here?

rhamzeh · 2025-03-16T13:15:30Z

  const max = Math.ceil(2 ** (loopCount + 1) * minWaitInMs);
-  return Math.floor(Math.random() * (max - min) + min); //The maximum is exclusive and the minimum is inclusive
+  const calculatedTime = Math.floor(Math.random() * (max - min) + min);
+  return Math.min(calculatedTime, 120000);


Suggested change

return Math.min(calculatedTime, 120000);

return Math.min(calculatedTime, {{maxBackoffTimeInSec}});

Defined here:

sdk-generator/config/common/config.base.json

Lines 88 to 89 in 885bdb7

"maxBackoffTimeInSec": 120,

"retryHeaderMaxAllowableDurationInSec": 1800,

rhamzeh · 2025-03-16T13:17:25Z

        throw new FgaApiNotFoundError(err);
-      } else if (status === 429 || status >= 500) {
-        if (iterationCount >= config.maxRetry) {
+      } else if (status === 429 || (status >= 500 && status !== 501)) {


Can you also make sure we are retrying on network errors?

Currently we do not retry on them: https://github.com/openfga/js-sdk/blob/8fb95c80ff57d10616c0fd60ca1dfc991ce41675/common.ts#L177

coderabbitai · 2025-10-08T14:31:23Z

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.

Pre-merge checks and finishing touches

✅ 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 clearly states the addition of Retry-After header support in the JS SDK and follows the conventional commit style. It accurately captures the core feature introduced in the pull request without extraneous information. This conciseness ensures that teammates can immediately understand the primary change.
Linked Issues Check	✅ Passed	The changes introduce structured retry logic that honors the Retry-After header, falls back to exponential backoff with configurable minWait and maxRetry, and implement specific handling for 429s, 5xxs, and network errors while dropping X-Rate-Limit-Reset, matching the objectives of issue #208. Tests were added to cover header parsing and backoff behavior and internal helper logic parses and validates the Retry-After header. Although default values and jitter bounds are inferred via minWaitInMs and config, the summary demonstrates coverage of the coding requirements.
Out of Scope Changes Check	✅ Passed	All modifications relate to implementing and testing the enhanced retry strategy defined in issue #208, including parser logic, backoff behavior, and test mocks for headers, and no unrelated functionality was introduced. Reorganization of test structures and header parameter support in mocks facilitate the retry-after tests and do not add features outside the linked objectives. The changes in public exports align with the new retry API surface without unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

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

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2a6003 and e12cc58.

📒 Files selected for processing (3)

config/clients/js/template/common.mustache (2 hunks)
config/clients/js/template/tests/helpers/nocks.ts.mustache (1 hunks)
config/clients/js/template/tests/index.test.ts.mustache (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

config/**/*.mustache

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Validate mustache syntax and variable references across all template files, including CHANGELOG.md.mustache

Files:

config/clients/js/template/tests/helpers/nocks.ts.mustache
config/clients/js/template/tests/index.test.ts.mustache
config/clients/js/template/common.mustache

config/**/*.{json,mustache}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Never hardcode API keys or credentials in configuration or template files

Files:

config/clients/js/template/tests/helpers/nocks.ts.mustache
config/clients/js/template/tests/index.test.ts.mustache
config/clients/js/template/common.mustache

🧠 Learnings (1)

📚 Learning: 2025-08-12T14:18:58.827Z

Learnt from: rhamzeh
PR: openfga/sdk-generator#573
File: config/clients/java/config.overrides.json:184-191
Timestamp: 2025-08-12T14:18:58.827Z
Learning: In the OpenFGA SDK generator, templates are inherited from the upstream OpenAPI Generator (openapitools/openapi-generator-cli). Only custom or modified templates need to be explicitly defined in the config overrides files. Base templates like `settings.gradle.mustache` are provided by the upstream generator and don't require override mappings.

Applied to files:

config/clients/js/template/common.mustache

🪛 Gitleaks (8.28.0)

config/clients/js/template/tests/index.test.ts.mustache

[high] 881-881: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

ryanpq marked this pull request as ready for review March 5, 2025 17:42

ryanpq requested a review from a team as a code owner March 5, 2025 17:42

rhamzeh reviewed Mar 16, 2025

View reviewed changes

rhamzeh marked this pull request as draft April 28, 2025 15:33

ryanpq added 2 commits October 9, 2025 08:19

feat(js-sdk): support retry-after header

ed34b98

feat(js-sdk): Add test for honoring retry-after header

6bc19e4

SoulPancake force-pushed the feat(js-sdk)-support-retry-after-header branch from e5beab7 to 6bc19e4 Compare October 9, 2025 02:49

SoulPancake added 2 commits October 9, 2025 21:41

Merge branch 'main' into feat(js-sdk)-support-retry-after-header

801255e

feat: address comments

e12cc58

SoulPancake mentioned this pull request Oct 9, 2025

feat: add support for handling Retry-After header openfga/js-sdk#267

Merged

4 tasks

SoulPancake marked this pull request as ready for review October 9, 2025 18:32

SoulPancake requested a review from rhamzeh October 9, 2025 18:32

coderabbitai Bot reviewed Oct 9, 2025

View reviewed changes

Comment thread config/clients/js/template/common.mustache

Comment thread config/clients/js/template/common.mustache

feat: address coderabbit comments

1a8d2ba

SoulPancake closed this Oct 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(js-sdk): support retry-after header#504

feat(js-sdk): support retry-after header#504
ryanpq wants to merge 5 commits into
mainfrom
feat(js-sdk)-support-retry-after-header

ryanpq commented Mar 4, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025

Uh oh!

rhamzeh Mar 16, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 8, 2025 •

edited

Loading

Review skipped

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (msValue >= 1000 && msValue <= 1800000) {
	if (msValue > 0 && msValue <= {{retryHeaderMaxAllowableDurationInSec}}) {
	}

	return Math.min(calculatedTime, 120000);
	return Math.min(calculatedTime, {{maxBackoffTimeInSec}});

	"maxBackoffTimeInSec": 120,
	"retryHeaderMaxAllowableDurationInSec": 1800,

Uh oh!

Conversation

ryanpq commented Mar 4, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

References

Review Checklist

Summary by CodeRabbit

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

rhamzeh Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ryanpq commented Mar 4, 2025 •

edited by coderabbitai Bot

Loading

rhamzeh Mar 16, 2025 •

edited

Loading

coderabbitai Bot commented Oct 8, 2025 •

edited

Loading