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.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 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? |
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.40% +0.60%
==========================================
Files 26 26
Lines 1268 1339 +71
Branches 225 277 +52
==========================================
+ Hits 1088 1157 +69
- Misses 110 115 +5
+ Partials 70 67 -3 ☔ 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
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
…T" to avoid undefined request method, streaming timeout kills long-running streams
…cts. With fetch, createStreamingRequestFunction returns the raw ReadableStream directly — $response is never set, so the fallback was the only path that ever executed.
…rror properties Overwriting .constructor and .name on an error doesn't actually change its type, so instanceof checks failed for non-401/403 token endpoint errors.
| (err as any).clientId = clientCredentials.clientId; | ||
| (err as any).audience = clientCredentials.apiAudience; | ||
| (err as any).grantType = "client_credentials"; | ||
| const authErr = new FgaApiAuthenticationError({ |
There was a problem hiding this comment.
My only issue with this is that we're losing the stack trace for the rethrown error.
I believe this is the better approach: https://github.com/openfga/js-sdk/pull/329/changes
|
It would be awesome to get this one in/released since axios 1.15 currently has 10+ security alerts (and had even bigger issues earlier this year). Btw huge fans of OpenFGA - thanks for building it! |
Thanks for your kind words @derekcicerone! We're 100% aiming to getting it merged soon, just some minor tweaks to be made first. That said, don't expect it out this week, we intend for it to go out with another branch we're working on - ESM Modules. As both are pretty big changes, we'll release both in the same release. Hopefully it will be out soon though! 🤞🏽 |
|
In the meantime, we'll bump axios to v1.16.0 and push out a release - thanks for the heads up. https://github.com/axios/axios/releases/tag/v1.15.1 and https://github.com/axios/axios/releases/tag/v1.15.2 both have security notices neither dependabot, nor snyk or socket.dev flagged for us - thank you for the heads up! |
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