feat: add streamedListObjects method with cross-platform support #278
Conversation
|
|
|
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 WalkthroughThis PR introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as OpenFgaClient
participant Detector as isNodeEnvironment()
participant NodePath as parseNDJSONStream
participant BrowserPath as parseNDJSONReadableStream
participant API as OpenFGA API
participant Consumer as Application
Consumer->>Client: streamedListObjects(request)
Client->>Detector: Detect environment
alt Node.js Environment
Detector-->>Client: true
Client->>API: axios GET /streamed-list-objects (streaming)
API-->>NodePath: Response stream
NodePath->>NodePath: Buffer chunks, split on newline
NodePath->>NodePath: Parse each line as JSON
loop For each valid object
NodePath-->>Consumer: yield StreamedListObjectsResponse
end
else Browser Environment
Detector-->>Client: false
Client->>API: fetch GET /streamed-list-objects
API-->>BrowserPath: Response.body (ReadableStream)
BrowserPath->>BrowserPath: Read chunks with TextDecoder
BrowserPath->>BrowserPath: Buffer & split on newline
BrowserPath->>BrowserPath: Parse each line as JSON
loop For each valid object
BrowserPath-->>Consumer: yield StreamedListObjectsResponse
end
BrowserPath->>BrowserPath: Release reader lock
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
[pre_merge_check_pass] The PR successfully addresses all coding-related objectives specified in issue #236. It implements a new streamedListObjects method in OpenFgaClient that calls the /stores/{store_id}/streamed-list-objects endpoint [client.ts], auto-detects the runtime environment to select appropriate streaming mechanisms for both Node.js and browser platforms [streaming.ts], parses NDJSON responses through AsyncGenerator to yield results incrementally without the previous 1000-object limit, and provides the same interface pattern as the Python SDK v0.9.1 implementation. The feature is properly exported from index.ts, fully tested with 13 new streaming tests, and documented with working examples and a README. | 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 #278 +/- ##
==========================================
- Coverage 88.88% 84.77% -4.12%
==========================================
Files 23 24 +1
Lines 1260 1484 +224
Branches 230 279 +49
==========================================
+ Hits 1120 1258 +138
- Misses 84 160 +76
- Partials 56 66 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (5)
streaming.ts (2)
21-58: Consider allowing users to configure error logging behavior.The function uses
console.warnto log parse errors, which directly writes to the console without giving library consumers control over logging. For a library, consider:
- Accepting an optional error callback parameter
- Using a configurable logger instance
- Throwing errors instead of silently continuing (with clear documentation)
- Or at minimum, making the logging behavior opt-in
This would allow consuming applications to integrate with their own logging infrastructure and handle parse errors according to their requirements.
</review_comment_end>
65-112: Consider extracting common parsing logic to reduce duplication.The line parsing logic (lines 88-98) is nearly identical to lines 35-47 in
parseNDJSONStream. The buffer handling, line splitting, trimming, JSON parsing, and error handling are duplicated.Consider extracting a shared helper function for the common parsing logic:
function* parseLines(lines: string[]): Generator<any> { for (const line of lines) { const trimmed = line.trim(); if (trimmed) { try { yield JSON.parse(trimmed); } catch (err) { console.warn("Failed to parse JSON line:", err); } } } }This would make both parsers more maintainable and ensure consistent behavior.
</review_comment_end>
client.ts (3)
851-857: Consider setting the client method header for observability.Other API methods in this client use
setHeaderIfNotSet(headers, CLIENT_METHOD_HEADER, "MethodName")for tracking and debugging purposes. Consider adding:const accessTokenHeader = await this.credentials.getAccessTokenHeader(); const headers: Record<string, string> = { ...options.headers }; if (accessTokenHeader) { headers[accessTokenHeader.name] = accessTokenHeader.value; } headers["Content-Type"] = "application/json"; +setHeaderIfNotSet(headers, CLIENT_METHOD_HEADER, "StreamedListObjects");This would maintain consistency with other methods like
listObjects(line 809),check(line 621), etc.</review_comment_end>
872-876: Validate the NDJSON response structure before accessing nested properties.The code assumes
item.result.objectexists without validation. If the server returns a different structure or a malformed line is parsed, this could silently skip valid data or fail unexpectedly.Consider adding structure validation:
for await (const item of parseNDJSONStream(response.data)) { - if (item && item.result && item.result.object) { + if (item?.result?.object && typeof item.result.object === 'string') { yield { object: item.result.object } as StreamedListObjectsResponse; + } else if (item) { + console.warn("Unexpected NDJSON item structure:", item); } }This adds type checking and provides visibility into structural mismatches.
</review_comment_end>
894-898: Avoid code duplication in NDJSON parsing logic.The browser path (lines 894-898) duplicates the exact same filtering logic from the Node path (lines 872-876). If the validation logic needs to change (e.g., to add error handling), it would need to be updated in both places.
Consider extracting this into a helper function:
private validateAndYieldStreamedObject(item: any): StreamedListObjectsResponse | null { if (item?.result?.object && typeof item.result.object === 'string') { return { object: item.result.object } as StreamedListObjectsResponse; } return null; }Then use it in both paths:
for await (const item of parseNDJSONStream(response.data)) { const result = this.validateAndYieldStreamedObject(item); if (result) yield result; }</review_comment_end>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md(1 hunks)apiModel.ts(1 hunks)client.ts(10 hunks)example/streamed-list-objects/README.md(1 hunks)example/streamed-list-objects/model.json(1 hunks)example/streamed-list-objects/streamedListObjects.mjs(1 hunks)index.ts(1 hunks)streaming.ts(1 hunks)tests/client.test.ts(5 hunks)tests/helpers/nocks.ts(7 hunks)tests/streaming.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
tests/streaming.test.ts (1)
streaming.ts (3)
isNodeEnvironment(118-120)parseNDJSONStream(21-58)parseNDJSONReadableStream(65-112)
example/streamed-list-objects/streamedListObjects.mjs (1)
client.ts (5)
createStore(336-338)writeTuples(580-584)streamedListObjects(837-900)listObjects(809-819)OpenFgaClient(245-1000)
tests/helpers/nocks.ts (3)
apiModel.ts (4)
ReadRequest(1172-1197)ExpandRequest(597-622)ListUsersRequest(900-943)ListUsersResponse(951-958)tests/helpers/default-config.ts (1)
defaultConfiguration(69-69)tests/helpers/index.ts (1)
defaultConfiguration(15-15)
streaming.ts (1)
index.ts (3)
parseNDJSONStream(27-27)parseNDJSONReadableStream(27-27)isNodeEnvironment(27-27)
client.ts (2)
apiModel.ts (3)
TupleKey(1445-1470)ListObjectsRequest(804-847)StreamedListObjectsResponse(868-875)streaming.ts (3)
isNodeEnvironment(118-120)parseNDJSONStream(21-58)parseNDJSONReadableStream(65-112)
tests/client.test.ts (1)
tests/helpers/default-config.ts (2)
baseConfig(54-67)defaultConfiguration(69-69)
🔇 Additional comments (15)
CHANGELOG.md (1)
7-10: LGTM! Clear and comprehensive changelog entry.The changelog entry accurately documents the new streaming capability with appropriate technical details about environment detection and the benefit of surpassing the 1000-object limit.
example/streamed-list-objects/model.json (1)
1-251: LGTM! Well-structured authorization model.The model definition follows OpenFGA schema conventions and provides a comprehensive example with multiple types (user, group, folder, document) and their relationships for demonstrating the streaming functionality.
tests/helpers/nocks.ts (1)
260-275: LGTM! Proper NDJSON mock implementation.The
streamedListObjectshelper correctly constructs a newline-delimited JSON response matching the expected streaming API format. The NDJSON structure{"result":{"object":"..."}}aligns with the response interface, and the trailing newline on line 268 follows NDJSON conventions.apiModel.ts (1)
863-875: LGTM! Clean interface definition.The
StreamedListObjectsResponseinterface is properly defined with appropriate documentation and follows the existing patterns in the codebase. The singleobject: stringfield correctly represents an individual streamed object result.index.ts (1)
27-27: LGTM! Appropriate public API expansion.The new exports make the streaming utilities available to SDK consumers, which is useful for advanced use cases where developers might need direct access to the NDJSON parsing functions or environment detection.
example/streamed-list-objects/README.md (1)
1-34: LGTM! Comprehensive example documentation.The README provides clear prerequisites, setup instructions, and explains the value proposition of the streaming endpoint. The step-by-step guide makes it easy for developers to understand and run the example.
tests/client.test.ts (2)
302-302: LGTM! Cleanup of unnecessary parameter.Removing the empty string argument improves code clarity without changing the behavior of the
readChangescall.
826-851: LGTM! Well-structured streaming test.The test properly verifies the
streamedListObjectsfunctionality using the async generator pattern. It correctly:
- Sets up mock data with the nocks helper
- Uses
for await...ofto consume the generator- Verifies both the count and content of streamed results
tests/streaming.test.ts (3)
17-22: LGTM! Environment detection test.The test correctly verifies that
isNodeEnvironment()returns true in the Node.js test environment.
24-102: LGTM! Comprehensive Node.js streaming tests.The test suite thoroughly covers the
parseNDJSONStreamfunction with:
- Single and multiple line NDJSON parsing
- Chunked data handling (critical for streaming reliability)
- Empty line handling
- Error recovery with invalid JSON (properly mocking and restoring console.warn)
These tests ensure robust behavior across various real-world streaming scenarios.
104-234: LGTM! Excellent browser streaming test coverage.The
parseNDJSONReadableStreamtests mirror the Node.js tests and add browser-specific scenarios:
- The
createMockReadableStreamhelper is well-designed for simulating browser streams- The reader lock release test (lines 203-214) is particularly important for preventing resource leaks in browser environments
- The incomplete data handling test (lines 216-233) ensures graceful degradation
The comprehensive coverage gives confidence in cross-platform streaming reliability.
streaming.ts (1)
118-120: LGTM!The environment detection logic is sound. Checking both the type of
processand the presence ofprocess.versions.nodewith optional chaining handles edge cases well.</review_comment_end>
example/streamed-list-objects/streamedListObjects.mjs (1)
1-106: LGTM!The example clearly demonstrates the streaming feature with proper error handling and cleanup. The comparison between streaming and standard endpoints effectively illustrates the benefit of the new feature.
</review_comment_end>
client.ts (2)
37-37: LGTM!The new imports for streaming support are correctly added and properly used in the
streamedListObjectsmethod.</review_comment_end>
Also applies to: 58-58
821-836: LGTM!The JSDoc documentation clearly explains the streaming behavior and cross-platform support. The return type
AsyncGenerator<StreamedListObjectsResponse>correctly reflects the streaming nature of the API.</review_comment_end>
- Export calculateExponentialBackoffWithJitter from common.mustache for reuse - Add SDK-specific error types for browser fetch requests - Add retry logic for browser with exponential backoff - Remove all inline imports from templates - Reuse SDK's standard backoff function (DRY principle) - Add 7 new tests for error handling and retry logic - Clean adapter pattern for Fetch API errors - Update CHANGELOG Addresses CodeRabbit feedback on openfga/js-sdk#278 Generated SDK: 139 tests pass, 85.88% streaming coverage
- Export calculateExponentialBackoffWithJitter from common.ts for reuse - Add SDK-specific error types for browser fetch (FgaApiValidationError, etc.) - Add retry logic for browser requests with exponential backoff - Remove all inline imports (moved to top of files) - Reuse SDK's standard backoff function (DRY principle) - Add 7 new tests for error handling and retry logic - Clean adapter pattern for mapping Fetch Response to SDK errors - Update CHANGELOG Addresses CodeRabbit feedback on PR #278 Tests: 157/157 passing, 85.88% streaming coverage
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…telemetry - Node: axios stream path via API layer; unwraps to raw Readable - Browser: fetch + NDJSON ReadableStream parser with exponential backoff & jitter - Unified error handling across axios/fetch; maps to SDK error classes - Robust NDJSON parsing (async-iterable and event-based fallback) - New examples: streamed-list-objects and streamed-list-objects-local (FGA_API_URL) - Tests and nock stream mocks; generator templates updated to include streaming helper - CHANGELOG and PR summaries updated
daniel-jonathan
force-pushed
the
feat/streaming-endpoint-list-objects
branch
from
1729c37 to
48808f8
Compare
2 participants
Summary
Adds
streamedListObjects(streaming ListObjects) for retrieving unlimited objects via the streaming API. Supports both Node.js and browser environments, with resilient NDJSON parsing and consistent telemetry.Fixes #236
Changes
streaming.tswith NDJSON parsers for Node.js Readable and browser ReadableStreamStreamedListObjectsResponseinterface toapiModel.tsOpenFgaClient.streamedListObjects()async generatorcalculateExponentialBackoffWithJitter()fromcommon.tsfor reuseindex.tsexample/streamed-list-objects(model-based, end-to-end streaming)example/streamed-list-objects-local(local server, minimal model)Features
Usage Example
Testing
Live testing:
Commits
Technical Details
responseType: 'stream'; returns raw stream from API layerReadableStreamAlignment with Python SDK
Related
Checklist