Open
Conversation
The global fetch API is built into Node 20+, browsers, Deno, Cloudflare Workers, and Vercel Edge. On modern versions of node taht use undici internally, this should result in improved perfomance. BREAKING CHANGE: The `$response` property type changes from `AxiosResponse<T>` to `FgaResponse<T>`. The constructor now accepts an optional `HttpClient` instead of `AxiosInstance`. `baseOptions.httpAgent`/`httpsAgent` are no longer applicable as fetch handles connection pooling natively.
WalkthroughThis PR replaces axios with a fetch-based HttpClient across the SDK, introducing Changes
Sequence DiagramsequenceDiagram
participant Client as OpenFgaClient
participant API as OpenFgaApi / executeApiRequest
participant HC as HttpClient
participant Fetch as globalThis.fetch
participant Retry as RetryLogic
Client->>API: call API method (options, httpClient?)
API->>HC: resolve injected or globalHttpClient
API->>Retry: execute attemptHttpRequest(config, hc)
Retry->>HC: merge headers, build FetchRequestConfig (method, body, timeout, responseType)
HC->>Fetch: fetch(url, {method, headers, body, signal})
alt Success 2xx
Fetch->>HC: Response
HC->>API: parse JSON/text or return stream -> FgaResponse{status,statusText,headers,data}
API->>Client: resolve CallResult with $response: FgaResponse
else Retryable (429/5xx/network)
Fetch->>Retry: Error/status
Retry->>Retry: backoff and retry attempts
Retry->>HC: re-attempt fetch
else Non-retryable (4xx, auth, not-found)
Fetch->>HC: Response
HC->>API: build HttpErrorContext and throw typed FgaApiError subclass
API->>Client: reject with error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: OpenFGA's Space StreamedListObjects Feature OverviewView Suggested Changes@@ -115,7 +115,7 @@
### JS SDK
The JS SDK supports StreamedListObjects in v0.9.3 and later. The method streams results using async iterators, allowing you to process each object as it arrives. See the [documentation](#streamed-list-objects) for usage details.
-**Runtime Requirements:** The JS SDK requires Node.js v20.19.0 or higher. Tested and supported versions include Node.js 20.x, 22.x, 24.x, and 25.x. For detailed information about supported Node.js versions and the support policy, see [SUPPORTED_RUNTIMES.md](https://github.com/openfga/js-sdk/blob/main/SUPPORTED_RUNTIMES.md).
+**Runtime Requirements:** The JS SDK requires a runtime with native `fetch` and `ReadableStream` support. Supported runtimes include Node.js 20+, modern browsers, Deno, Cloudflare Workers, and Vercel Edge. The primary CI-tested Node.js versions are 20.x, 22.x, 24.x, and 25.x. For detailed information about supported runtimes and the support policy, see [SUPPORTED_RUNTIMES.md](https://github.com/openfga/js-sdk/blob/main/SUPPORTED_RUNTIMES.md).
Additionally, the JS SDK provides `executeApiRequest` and `executeStreamedApiRequest` methods on `OpenFgaClient` that can be used to call arbitrary API endpoints that don't yet have dedicated SDK methods. Use `executeApiRequest` for regular endpoints and `executeStreamedApiRequest` for streaming endpoints like `/streamed-list-objects`. These methods act as an escape hatch while still providing SDK benefits like authentication, configuration, retries, and error handling.
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the SDK’s transport layer from Axios to the native fetch API to improve cross-runtime support (Node.js, browsers, Deno/edge-like environments) and reduce dependencies, while updating the public response surface ($response) and HTTP injection mechanism.
Changes:
- Replaces Axios usage with a fetch-based
HttpClientabstraction and introducesFgaResponse<T>as the new$responsetype. - Refactors retry/timeout/error handling to work with
fetch+AbortSignal.timeout(), including streamed responses viaReadableStream. - Updates docs and tests to reflect the new HTTP stack and breaking API changes.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
common.ts |
Core migration: implements fetch-based request execution, retries, response parsing, and new HttpClient/FgaResponse types. |
api.ts |
Updates generated API surface to accept HttpClient injection and uses fetch-based request functions. |
base.ts |
Replaces Axios setup with HttpClient initialization and passes it into credentials/API layers. |
client.ts |
Switches client $response types to FgaResponse and adjusts streaming behavior to ReadableStream. |
errors.ts |
Decouples errors from Axios by introducing HttpErrorContext and updating error constructors. |
credentials/credentials.ts |
Migrates token exchange to fetch-based requests via HttpClient. |
configuration.ts |
Tightens typing for baseOptions and updates comments to reflect HTTP (not Axios) semantics. |
index.ts |
Re-exports new HTTP/response types from common.ts. |
package.json |
Removes axios dependency. |
tests/fetch-http-client.test.ts |
Adds focused tests for fetch client behavior (headers, retries, timeout, parsing, injection). |
tests/index.test.ts |
Updates retry/non-network error test to mock fetch instead of Axios. |
tests/errors-authentication.test.ts |
Updates authentication error test to use HttpErrorContext. |
README.md |
Documents custom HTTP client usage and the new $response type. |
SUPPORTED_RUNTIMES.md |
Broadens runtime support documentation beyond Node.js. |
CHANGELOG.md |
Records breaking changes and migration notes for the Axios → fetch switch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #366 +/- ##
==========================================
+ Coverage 85.80% 86.29% +0.48%
==========================================
Files 26 26
Lines 1268 1335 +67
Branches 225 274 +49
==========================================
+ Hits 1088 1152 +64
- Misses 110 115 +5
+ Partials 70 68 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
rhamzeh
force-pushed
the
feat/drop-axios
branch
from
7e2106f to
1fb03ec
Compare
rhamzeh
force-pushed
the
feat/drop-axios
branch
from
1fb03ec to
33d64f3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api.ts`:
- Around line 487-490: The FP streaming function signature is incorrect: update
executeStreamedApiRequest's returned inner function type from PromiseResult<any>
to Promise<any> to reflect that createStreamingRequestFunction (the
implementation) returns the raw response.data stream; likewise change the
OO/public streaming method's return type from Promise<CallResult<any>> to
Promise<any> so callers don't expect a $response property—locate the
executeStreamedApiRequest implementation that calls RequestBuilder and
createStreamingRequestFunction (and the public method that wraps it where
TelemetryAttribute.FgaClientRequestMethod is set) and adjust their declared
return types to Promise<any>.
In `@common.ts`:
- Around line 376-382: Replace the naive JSON check with the existing MIME
helper: instead of using contentType.includes("application/json") check the full
JSON MIME family by calling isJsonMime(contentType) (use the local contentType
variable and keep the existing branches that set data via response.json() or
response.text()); update the conditional in the block that inspects
response.headers/get("content-type") so responses like application/problem+json,
vendor+json, or mixed-case types are parsed with response.json().
- Around line 721-724: The call to attemptHttpRequest currently omits the
telemetry block so streaming requests (and retries) don't emit
histogramHttpRequestDuration; update the call where wrappedResponse is assigned
to pass the telemetry through (e.g., include telemetry in the options object
alongside maxRetry and minWaitInMs or as the appropriate parameter expected by
attemptHttpRequest) so attemptHttpRequest receives the telemetry and can emit
histogramHttpRequestDuration; reference the existing symbols attemptHttpRequest,
wrappedResponse, fetchRequestConfig, maxRetry, minWaitInMs, and client to locate
and modify the call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8225e0a3-58cf-42ce-9769-864c555633b7
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mdSUPPORTED_RUNTIMES.mdapi.tsbase.tsclient.tscommon.tsconfiguration.tscredentials/credentials.tserrors.tsindex.tspackage.jsontests/errors-authentication.test.tstests/fetch-http-client.test.tstests/index.test.ts
💤 Files with no reviewable changes (1)
- package.json
rhamzeh
force-pushed
the
feat/drop-axios
branch
from
e93244a to
f24608a
Compare
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rhamzeh
force-pushed
the
feat/drop-axios
branch
from
f24608a to
ef3a638
Compare
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common.ts`:
- Around line 750-773: createStreamingRequestFunction currently returns the raw
stream (response?.data) while client code (executeStreamedApiRequest and the
handler that checks stream.$response.data) expects a PromiseResult/$response
envelope; standardize by changing createStreamingRequestFunction to return the
same envelope shape as other requests (e.g., an object with $response containing
the full response and data as the stream), and
update/createStreamingRequestFunction's public signature/docs to
PromiseResult<any>, plus adjust any consumer code in client.ts that expects
stream.$response.data to use the unified envelope returned by
createStreamingRequestFunction (references: createStreamingRequestFunction,
executeStreamedApiRequest, and the handler that inspects stream.$response.data).
- Around line 306-320: The current logic always attaches an AbortSignal
(timeoutMs) which will abort even after fetch resolves and thus cuts off
streaming responses; modify the timeout creation to skip adding the implicit
default timeout when the request is a streamed response (check
request.responseType === "stream" or similar) unless the caller explicitly
provided request.timeout; i.e., only set signal when request.timeout is defined
or request.responseType !== "stream" (keep using timeoutMs, AbortSignal.timeout
fallback and AbortController fallback as before), and add a regression test that
issues a streaming request that remains open past the default 10s to ensure
streams are not aborted by HttpClient.defaultTimeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4bc75d66-f8e3-4796-90b1-84a6bc52480c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdSUPPORTED_RUNTIMES.mdapi.tsclient.tscommon.tserrors.tstests/errors.test.tstests/fetch-http-client.test.ts
✅ Files skipped from review due to trivial changes (4)
- SUPPORTED_RUNTIMES.md
- CHANGELOG.md
- README.md
- api.ts
Comment on lines
+306
to
+320
| const timeoutMs = request.timeout ?? httpClient.defaultTimeout ?? 10000; | ||
| let signal: AbortSignal | undefined; | ||
| if (typeof AbortSignal !== "undefined" && typeof (AbortSignal as any).timeout === "function") { | ||
| // Use native AbortSignal.timeout when available. | ||
| signal = (AbortSignal as any).timeout(timeoutMs); | ||
| } else if (typeof AbortController !== "undefined") { | ||
| // Fallback for environments without AbortSignal.timeout. | ||
| const controller = new AbortController(); | ||
| setTimeout(() => controller.abort(), timeoutMs); | ||
| signal = controller.signal; | ||
| } | ||
| const fetchInit: RequestInit = { | ||
| method: request.method || "GET", | ||
| headers: fetchHeaders, | ||
| signal, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "common.ts" | head -5Repository: openfga/js-sdk
Length of output: 69
🏁 Script executed:
wc -l ./common.tsRepository: openfga/js-sdk
Length of output: 73
🏁 Script executed:
sed -n '300,325p' ./common.tsRepository: openfga/js-sdk
Length of output: 1096
🏁 Script executed:
sed -n '405,420p' ./common.tsRepository: openfga/js-sdk
Length of output: 603
🏁 Script executed:
sed -n '260,290p' ./common.tsRepository: openfga/js-sdk
Length of output: 763
🏁 Script executed:
rg "responseType.*stream" ./common.ts -B 2 -A 2Repository: openfga/js-sdk
Length of output: 375
🏁 Script executed:
sed -n '290,345p' ./common.tsRepository: openfga/js-sdk
Length of output: 2374
🏁 Script executed:
rg "createStreamingRequestFunction\|responseType.*stream" ./common.ts -B 5 -A 5Repository: openfga/js-sdk
Length of output: 40
🏁 Script executed:
rg "responseType" ./common.ts -B 3 -A 3Repository: openfga/js-sdk
Length of output: 579
🏁 Script executed:
sed -n '435,455p' ./common.tsRepository: openfga/js-sdk
Length of output: 755
🏁 Script executed:
rg "streaming\|stream" ./common.ts -i | head -20Repository: openfga/js-sdk
Length of output: 40
🏁 Script executed:
sed -n '750,817p' ./common.tsRepository: openfga/js-sdk
Length of output: 2311
🏁 Script executed:
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "stream\|timeout" 2>/dev/null | head -10Repository: openfga/js-sdk
Length of output: 140
🏁 Script executed:
grep -n "timeout\|stream" ./tests/streaming.test.ts | head -30Repository: openfga/js-sdk
Length of output: 1119
🏁 Script executed:
grep -n "timeout" ./tests/streaming.test.tsRepository: openfga/js-sdk
Length of output: 40
🏁 Script executed:
grep -n "timeout" ./tests/fetch-http-client.test.ts -B 2 -A 5Repository: openfga/js-sdk
Length of output: 1059
Don't inherit the default 10s timeout into streamed bodies.
The abort signal created at line 306 remains attached after fetch() resolves, so responseType: "stream" requests will be cut off once HttpClient.defaultTimeout elapses. With the default client, a slow or large stream can fail after ~10 seconds even though the HTTP request itself succeeded. Skip the implicit timeout for streams unless the caller explicitly opts into one, and add a regression test that keeps a stream open past the default timeout.
Suggested direction
- const timeoutMs = request.timeout ?? httpClient.defaultTimeout ?? 10000;
+ const timeoutMs =
+ request.responseType === "stream" && request.timeout == null
+ ? undefined
+ : request.timeout ?? httpClient.defaultTimeout ?? 10000;
let signal: AbortSignal | undefined;
- if (typeof AbortSignal !== "undefined" && typeof (AbortSignal as any).timeout === "function") {
+ if (typeof timeoutMs === "number" && typeof AbortSignal !== "undefined" && typeof (AbortSignal as any).timeout === "function") {
// Use native AbortSignal.timeout when available.
signal = (AbortSignal as any).timeout(timeoutMs);
- } else if (typeof AbortController !== "undefined") {
+ } else if (typeof timeoutMs === "number" && typeof AbortController !== "undefined") {
// Fallback for environments without AbortSignal.timeout.
const controller = new AbortController();
setTimeout(() => controller.abort(), timeoutMs);
signal = controller.signal;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common.ts` around lines 306 - 320, The current logic always attaches an
AbortSignal (timeoutMs) which will abort even after fetch resolves and thus cuts
off streaming responses; modify the timeout creation to skip adding the implicit
default timeout when the request is a streamed response (check
request.responseType === "stream" or similar) unless the caller explicitly
provided request.timeout; i.e., only set signal when request.timeout is defined
or request.responseType !== "stream" (keep using timeoutMs, AbortSignal.timeout
fallback and AbortController fallback as before), and add a regression test that
issues a streaming request that remains open past the default 10s to ensure
streams are not aborted by HttpClient.defaultTimeout.
Comment on lines
+750
to
+773
| return async (client: HttpClient = httpClient): Promise<any> => { | ||
| await setBearerAuthToObject(axiosArgs.options.headers, credentials!); | ||
|
|
||
| const url = configuration.getBasePath() + axiosArgs.url; | ||
|
|
||
| const axiosRequestArgs = { ...axiosArgs.options, responseType: "stream", url: url }; | ||
| const wrappedResponse = await attemptHttpRequest(axiosRequestArgs, { | ||
| const fetchRequestConfig: FetchRequestConfig = { | ||
| url, | ||
| method: axiosArgs.options.method, | ||
| headers: axiosArgs.options.headers, | ||
| data: axiosArgs.options.data, | ||
| responseType: "stream", | ||
| timeout: axiosArgs.options.timeout, | ||
| }; | ||
|
|
||
| const wrappedResponse = await attemptHttpRequest(fetchRequestConfig, { | ||
| maxRetry, | ||
| minWaitInMs, | ||
| }, axios); | ||
| }, client, { | ||
| telemetry: configuration.telemetry, | ||
| userAgent: configuration.baseOptions?.headers?.["User-Agent"], | ||
| }); | ||
| const response = wrappedResponse?.response; | ||
|
|
||
| const result: any = response?.data; // raw stream | ||
| const result: any = response?.data; // raw ReadableStream |
There was a problem hiding this comment.
Keep one public return contract for streamed requests.
createStreamingRequestFunction() now returns response?.data directly, but client.ts Line 882 has to defensively handle both stream.$response.data and the raw stream, while executeStreamedApiRequest() at client.ts Lines 1066-1070 still advertises PromiseResult<any>. That leaves the streaming API with an unstable shape for consumers. Please standardize on one contract—either always return the $response envelope or update the public signatures/docs to return the raw stream explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common.ts` around lines 750 - 773, createStreamingRequestFunction currently
returns the raw stream (response?.data) while client code
(executeStreamedApiRequest and the handler that checks stream.$response.data)
expects a PromiseResult/$response envelope; standardize by changing
createStreamingRequestFunction to return the same envelope shape as other
requests (e.g., an object with $response containing the full response and data
as the stream), and update/createStreamingRequestFunction's public
signature/docs to PromiseResult<any>, plus adjust any consumer code in client.ts
that expects stream.$response.data to use the unified envelope returned by
createStreamingRequestFunction (references: createStreamingRequestFunction,
executeStreamedApiRequest, and the handler that inspects stream.$response.data).
SoulPancake
approved these changes
Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The global fetch API is built into Node 20+, browsers, Deno, Cloudflare Workers, and Vercel Edge.
On modern versions of node that use undici internally, this should result in improved performance.
Warning
BREAKING CHANGE: The
$responseproperty type changes fromAxiosResponse<T>toFgaResponse<T>. The constructor now accepts an optionalHttpClientinstead ofAxiosInstance.baseOptions.httpAgent/httpsAgentare no longer applicable as fetch handles connection pooling natively.Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
closes #18
unblocks #17
Review Checklist
mainSummary by CodeRabbit
Breaking Changes
New Features
Documentation
Chores