chore: improve api executor flow#345
Merged
Merged
Conversation
…quests * apiExecutor now has a consistent interface regardless of where called * apiExecutor now allows specifying types to help with typings and reduce casting * apiExecutor + runApiExecutor now centralize all retry + auth + telemetry logic * operationName is now required * add streamingApiExecutor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR refactors the OpenFgaClient's raw API call interface by replacing the internal Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
- Coverage 90.00% 88.51% -1.50%
==========================================
Files 25 25
Lines 1531 1271 -260
Branches 289 249 -40
==========================================
- Hits 1378 1125 -253
- Misses 95 101 +6
+ Partials 58 45 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Documentation Updates 2 document(s) were updated by changes in this PR: Custom HTTP Headers SupportView Changes@@ -177,7 +177,7 @@
```javascript
const response = await fgaClient.executeApiRequest({
- operationName: 'CustomEndpoint', // For telemetry/logging
+ operationName: 'CustomEndpoint', // Required for telemetry/logging
method: 'POST',
path: '/stores/{store_id}/custom-endpoint',
pathParams: { store_id: process.env.FGA_STORE_ID },
@@ -192,13 +192,17 @@
});
```
-You can pass custom headers in the `options` parameter. These headers are merged with global configuration headers, with per-request headers taking precedence:
+You can pass custom headers in either the `request.headers` parameter or the `options.headers` parameter. Headers from both sources are merged, with `options.headers` taking precedence:
```javascript
const response = await fgaClient.executeApiRequest({
+ operationName: 'GetAuthorizationModels',
method: 'GET',
path: '/stores/{store_id}/authorization-models',
pathParams: { store_id: 'your-store-id' },
+ headers: {
+ 'X-Request-Source': 'my-app',
+ }
}, {
headers: {
'X-Request-ID': '123e4567-e89b-12d3-a456-426614174000',StreamedListObjects Feature OverviewView Changes@@ -89,12 +89,13 @@
**Example:**
```javascript
const response = await fgaClient.executeApiRequest({
- operationName: 'CustomEndpoint',
+ operationName: 'CustomEndpoint', // Required: operation name for telemetry
method: 'POST',
path: '/stores/{store_id}/custom-endpoint',
pathParams: { store_id: process.env.FGA_STORE_ID },
body: { /* request body */ },
queryParams: { page_size: 20 },
+ headers: { 'X-Custom-Header': 'value' }, // Optional: custom headers
});
```
How did I do? Any feedback? |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the internal API execution flow to improve code organization and consistency. The main changes include:
Purpose: The PR consolidates the request-building logic into a unified RequestBuilder function and renames the public API method from apiExecutor to executeApiRequest (with a new executeStreamedApiRequest for streaming endpoints).
Changes:
- Introduced a centralized
RequestBuilderfunction incommon.tsthat handles path parameter substitution, query parameters, headers, and enforces SDK-required headers (Accept and Content-Type) - Renamed
apiExecutortoexecuteApiRequestand addedexecuteStreamedApiRequestfor streaming endpoints - Refactored all API methods in
OpenFgaApiFpto use the newexecuteApiRequestinternally, eliminating the oldOpenFgaApiAxiosParamCreatorpattern - Updated tests to reflect the new method names and verify proper header handling
- Updated documentation in README.md and CHANGELOG.md
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| common.ts | Added RequestBuilder function and related types (RequestBuilderParams, RequestBuilderOptions) to centralize request building logic |
| api.ts | Refactored all API methods to use executeApiRequest internally; removed OpenFgaApiAxiosParamCreator; added public executeApiRequest and executeStreamedApiRequest methods |
| client.ts | Renamed apiExecutor to executeApiRequest, added executeStreamedApiRequest, updated types and documentation |
| tests/apiExecutor.test.ts | Updated test suite to use new method names and added operationName parameter to all calls |
| tests/headers.test.ts | Updated test assertions to reflect that SDK now enforces both Content-Type and Accept headers |
| README.md | Updated documentation examples to use executeApiRequest instead of apiExecutor |
| CHANGELOG.md | Added entry for the new methods with documentation link |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/headers.test.ts (1)
466-490:⚠️ Potential issue | 🟡 MinorAdd an explicit
Acceptassertion in the protected-header test.This test title claims both
Content-TypeandAcceptare enforced, but onlyContent-Typeis currently asserted.Suggested fix
const scope = nock(testConfig.apiUrl!) .post(`/stores/${testConfig.storeId}/check`) .reply(function() { const headers = this.req.headers; // SDK enforces Content-Type and Accept for JSON APIs expect(headers["content-type"]).toBe("application/json"); + expect(headers["accept"]).toBe("application/json"); // Custom headers are passed through expect(headers["x-custom"]).toBe("definitely-works");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/headers.test.ts` around lines 466 - 490, The test "SDK enforces Content-Type and Accept regardless of baseOptions headers" currently asserts only Content-Type; update the nock reply handler in that test (inside the it block where fgaClient is created and scope is defined) to also assert that headers["accept"] === "application/json" so the test explicitly verifies the SDK enforces Accept for JSON APIs; locate the reply function that reads this.req.headers and add the Accept assertion alongside the existing Content-Type and x-custom assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 6: The documentation link in the CHANGELOG entry that mentions
executeApiRequest and executeStreamedApiRequest contains a broken fragment with
a double `##`; update the URL in that entry to use a single `#` (e.g., change
https://github.com/openfga/js-sdk##calling-other-endpoints to
https://github.com/openfga/js-sdk#calling-other-endpoints) so the anchor
resolves correctly.
In `@common.ts`:
- Around line 482-486: The current unresolved-parameter detection uses a regex
(requestPathTemplate.match(/\{([^}]+)\}/)) which triggers a CodeQL regex
complexity finding; replace it with a deterministic brace scan: use
requestPathTemplate.indexOf('{') to find the first opening brace and then
requestPathTemplate.indexOf('}', openIndex+1) to find the matching closing
brace, extract the parameter name via substring between those indexes, and if
both exist throw the same FgaValidationError(parameterName, `Path parameter
'${parameterName}' was not provided for path: ${request.path}`); update the
logic in the block that references requestPathTemplate and unresolvedMatch to
remove the regex and use this simple index-based extraction instead.
---
Outside diff comments:
In `@tests/headers.test.ts`:
- Around line 466-490: The test "SDK enforces Content-Type and Accept regardless
of baseOptions headers" currently asserts only Content-Type; update the nock
reply handler in that test (inside the it block where fgaClient is created and
scope is defined) to also assert that headers["accept"] === "application/json"
so the test explicitly verifies the SDK enforces Accept for JSON APIs; locate
the reply function that reads this.req.headers and add the Accept assertion
alongside the existing Content-Type and x-custom assertions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mdapi.tsclient.tscommon.tstests/apiExecutor.test.tstests/headers.test.ts
* Currently this affects ReadChanges method, and was the existing behaviour before the refactor
rhamzeh
force-pushed
the
chore/improve-api-executor
branch
from
February 27, 2026 02:48
75331e7 to
9aaeafd
Compare
SoulPancake
approved these changes
Feb 27, 2026
rhamzeh
added this pull request to the merge queue
Feb 27, 2026
4 tasks
4 tasks
4 tasks
4 tasks
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.
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Improvements