feat(client): integrate OpenAPI generated client - #2347
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe client migrates coordinator HTTP and WebSocket interactions to generated v2 APIs, adds compatibility exports for renamed models and enums, updates call and device operations, and adjusts SDK consumers and tests for the new request paths and types. ChangesCoordinator v2 migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StreamVideoClient
participant VideoApi
participant ApiClient
participant StreamClient
StreamVideoClient->>VideoApi: queryCalls or listDevices
VideoApi->>ApiClient: sendRequest
ApiClient->>StreamClient: doAxiosRequest(v2 endpoint)
StreamClient-->>ApiClient: AxiosResponse
ApiClient-->>VideoApi: response body and metadata
VideoApi-->>StreamVideoClient: typed result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/client/src/compat.ts (1)
27-449: 📐 Maintainability & Code Quality | 🔵 TrivialHand-duplicated enum values risk silent drift from the generated spec.
These enum objects (e.g. the ~34-entry language lists repeated across
StartClosedCaptionsRequestLanguageEnum,StartTranscriptionRequestLanguageEnum,TranscriptionSettingsRequestLanguageEnum,TranscriptionSettingsResponseLanguageEnum) are now manually maintained duplicates of what the generator used to produce. Sincegenerate-openapi.shregeneratessrc/gen/coordinatorfrom the spec but never touchescompat.ts, a future backend enum change (new language, new recording mode, etc.) can silently desync this compatibility layer from the real contract.Consider a lightweight guard — e.g. a unit test that asserts each compat enum's value set is a subset of/matches the corresponding inline literal union types used in the generated models — so drift fails CI instead of failing silently at runtime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/client/src/compat.ts` around lines 27 - 449, Add a unit test covering the compatibility enum objects in compat.ts, including the repeated language enums such as StartClosedCaptionsRequestLanguageEnum and TranscriptionSettingsResponseLanguageEnum, and compare each value set with its corresponding generated model literal union. Fail the test when values are missing or extra so regenerated coordinator specs cannot silently drift from the compatibility layer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client/src/Call.ts`:
- Around line 957-961: Remove the debug console.log call from getOrCreate after
the API response is retrieved; leave the existing setup, request, and response
flow unchanged.
In `@packages/client/src/coordinator/connection/api-client.ts`:
- Around line 77-100: Update queryParamsStringify so arrays containing objects
are serialized as structured JSON rather than via value.join(','). Preserve the
existing comma-joined serialization for primitive arrays, while ensuring
object-containing arrays produce usable encoded JSON output.
In `@packages/client/src/coordinator/connection/client.ts`:
- Line 125: Update Call.getCallStatsMap to preserve the /video path segment when
constructing its call_stats URL, matching the new baseURL behavior and the
existing Call.streamClientBasePath convention. Ensure the request still targets
/video/call_stats/${type}/${id}/${callSessionId}/map, and inspect other direct
streamClient.baseURL concatenations for the same missing segment.
In `@packages/client/src/store/CallState.ts`:
- Around line 343-344: Add the missing call.user_muted property to the
eventHandlers initializer in CallState, using an explicit undefined handler or
the intended state-update callback so it satisfies the required WSEvent mapped
type. Preserve all existing event handlers.
In `@packages/react-sdk/src/embedded/hooks/useNoiseCancellationLoader.ts`:
- Around line 20-22: Update useNoiseCancellationLoader to obtain call settings
via useCallStateHooks().useCallSettings() from `@stream-io/video-react-bindings`,
then derive isNoiseCancellationEnabled from that reactive value. Remove the
direct call?.state.settings access while preserving the existing
noise-cancellation mode check and loader behavior.
---
Nitpick comments:
In `@packages/client/src/compat.ts`:
- Around line 27-449: Add a unit test covering the compatibility enum objects in
compat.ts, including the repeated language enums such as
StartClosedCaptionsRequestLanguageEnum and
TranscriptionSettingsResponseLanguageEnum, and compare each value set with its
corresponding generated model literal union. Fail the test when values are
missing or extra so regenerated coordinator specs cannot silently drift from the
compatibility layer.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 356568de-0880-458e-bb8c-a18eadd5180b
⛔ Files ignored due to path filters (7)
packages/client/src/gen/coordinator/index.tsis excluded by!**/gen/**packages/client/src/gen/coordinator/model-decoders/decoders.tsis excluded by!**/gen/**packages/client/src/gen/coordinator/model-decoders/event-decoder-mapping.tsis excluded by!**/gen/**packages/client/src/gen/coordinator/models/index.tsis excluded by!**/gen/**packages/client/src/gen/coordinator/video/CallApi.tsis excluded by!**/gen/**packages/client/src/gen/coordinator/video/VideoApi.tsis excluded by!**/gen/**packages/client/src/gen/gen-imports.tsis excluded by!**/gen/**
📒 Files selected for processing (22)
packages/client/generate-openapi.shpackages/client/index.tspackages/client/src/Call.tspackages/client/src/StreamVideoClient.tspackages/client/src/__tests__/StreamVideoClient.api.test.tspackages/client/src/compat.tspackages/client/src/coordinator/connection/api-client.tspackages/client/src/coordinator/connection/client.tspackages/client/src/coordinator/connection/connection.tspackages/client/src/coordinator/connection/types.tspackages/client/src/devices/MicrophoneManager.tspackages/client/src/devices/SpeakerManager.tspackages/client/src/reporting/ClientEventReporter.tspackages/client/src/rtc/helpers/rtcConfiguration.tspackages/client/src/store/CallState.tspackages/client/src/types.tspackages/react-native-sdk/src/components/Call/CallControls/internal/ReactionsPicker.tsxpackages/react-native-sdk/src/providers/NoiseCancellation/NoiseCancellationProvider.tsxpackages/react-sdk/src/components/NoiseCancellation/NoiseCancellationProvider.tsxpackages/react-sdk/src/core/components/ParticipantView/ParticipantActionsContextMenu.tsxpackages/react-sdk/src/embedded/hooks/useNoiseCancellationLoader.tssample-apps/react-native/dogfood/src/components/CallControls/MoreActionsButton/BottomControlsDrawer.tsx
| const queryParamsStringify = (params: Record<string, unknown>): string => { | ||
| const queryParams: string[] = []; | ||
|
|
||
| for (const key in params) { | ||
| const value = params[key]; | ||
| if (value == null) continue; | ||
|
|
||
| if (Array.isArray(value)) { | ||
| queryParams.push(`${key}=${encodeURIComponent(value.join(','))}`); | ||
| } else if (value instanceof Date) { | ||
| queryParams.push(`${key}=${encodeURIComponent(value.toISOString())}`); | ||
| } else if (typeof value === 'object') { | ||
| queryParams.push(`${key}=${encodeURIComponent(JSON.stringify(value))}`); | ||
| } else if ( | ||
| typeof value === 'string' || | ||
| typeof value === 'number' || | ||
| typeof value === 'boolean' | ||
| ) { | ||
| queryParams.push(`${key}=${encodeURIComponent(value)}`); | ||
| } | ||
| } | ||
|
|
||
| return queryParams.join('&'); | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Array-of-objects query params get silently corrupted.
Array.isArray(value) is checked before the generic object branch, so an array whose elements are themselves objects falls into value.join(','), producing "[object Object],[object Object]" rather than a usable serialization. No current call site in this cohort appears to hit this, but it's a foot-gun for any future/edge-case usage of this shared serializer.
🛠️ Suggested fix
if (Array.isArray(value)) {
- queryParams.push(`${key}=${encodeURIComponent(value.join(','))}`);
+ const serialized = value.every((v) => v == null || typeof v !== 'object')
+ ? value.join(',')
+ : JSON.stringify(value);
+ queryParams.push(`${key}=${encodeURIComponent(serialized)}`);
} else if (value instanceof Date) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const queryParamsStringify = (params: Record<string, unknown>): string => { | |
| const queryParams: string[] = []; | |
| for (const key in params) { | |
| const value = params[key]; | |
| if (value == null) continue; | |
| if (Array.isArray(value)) { | |
| queryParams.push(`${key}=${encodeURIComponent(value.join(','))}`); | |
| } else if (value instanceof Date) { | |
| queryParams.push(`${key}=${encodeURIComponent(value.toISOString())}`); | |
| } else if (typeof value === 'object') { | |
| queryParams.push(`${key}=${encodeURIComponent(JSON.stringify(value))}`); | |
| } else if ( | |
| typeof value === 'string' || | |
| typeof value === 'number' || | |
| typeof value === 'boolean' | |
| ) { | |
| queryParams.push(`${key}=${encodeURIComponent(value)}`); | |
| } | |
| } | |
| return queryParams.join('&'); | |
| }; | |
| const queryParamsStringify = (params: Record<string, unknown>): string => { | |
| const queryParams: string[] = []; | |
| for (const key in params) { | |
| const value = params[key]; | |
| if (value == null) continue; | |
| if (Array.isArray(value)) { | |
| const serialized = value.every((v) => v == null || typeof v !== 'object') | |
| ? value.join(',') | |
| : JSON.stringify(value); | |
| queryParams.push(`${key}=${encodeURIComponent(serialized)}`); | |
| } else if (value instanceof Date) { | |
| queryParams.push(`${key}=${encodeURIComponent(value.toISOString())}`); | |
| } else if (typeof value === 'object') { | |
| queryParams.push(`${key}=${encodeURIComponent(JSON.stringify(value))}`); | |
| } else if ( | |
| typeof value === 'string' || | |
| typeof value === 'number' || | |
| typeof value === 'boolean' | |
| ) { | |
| queryParams.push(`${key}=${encodeURIComponent(value)}`); | |
| } | |
| } | |
| return queryParams.join('&'); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/coordinator/connection/api-client.ts` around lines 77 -
100, Update queryParamsStringify so arrays containing objects are serialized as
structured JSON rather than via value.join(','). Preserve the existing
comma-joined serialization for primitive arrays, while ensuring
object-containing arrays produce usable encoded JSON output.
| this.setBaseURL( | ||
| this.options.baseURL || 'https://video.stream-io-api.com/video', | ||
| ); | ||
| this.setBaseURL(this.options.baseURL || 'https://video.stream-io-api.com'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Base URL change breaks Call.getCallStatsMap, which manually reconstructs the URL.
Dropping the /video suffix from the default base URL is compensated for in connectGuestUser (now posts to /video/guest) and in Call.streamClientBasePath (now /video/call/${type}/${id}), but Call.getCallStatsMap was missed:
// packages/client/src/Call.ts (unchanged in this diff)
return this.streamClient.get<QueryCallStatsMapResponse>(
`${this.streamClient.baseURL}/call_stats/${this.type}/${this.id}/${callSessionId}/map`,
params,
);With the old base URL (.../video), this resolved to .../video/call_stats/.... With the new default (no /video suffix), it now resolves to .../call_stats/..., silently dropping the /video segment and almost certainly 404ing.
🐛 Suggested fix (in Call.ts)
return this.streamClient.get<QueryCallStatsMapResponse>(
- `${this.streamClient.baseURL}/call_stats/${this.type}/${this.id}/${callSessionId}/map`,
+ `${this.streamClient.baseURL}/video/call_stats/${this.type}/${this.id}/${callSessionId}/map`,
params,
);🔍 Verification script — check for other direct `baseURL` string concatenations
#!/bin/bash
rg -n 'streamClient\.baseURL' packages/client/src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/coordinator/connection/client.ts` at line 125, Update
Call.getCallStatsMap to preserve the /video path segment when constructing its
call_stats URL, matching the new baseURL behavior and the existing
Call.streamClientBasePath convention. Ensure the request still targets
/video/call_stats/${type}/${id}/${callSessionId}/map, and inspect other direct
streamClient.baseURL concatenations for the same missing segment.
| [EventType in WSEvent['type']]: | ||
| | ((event: Extract<WSEvent, { type: EventType }>) => void) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Add the missing call.user_muted handler key.
eventHandlers is now a required mapped type over WSEvent['type'], and generated WSEvent includes call.user_muted. The initializer has no corresponding property, so TypeScript reports a missing required key. Add an explicit undefined handler or implement the intended state update.
Proposed fix
'call.user_feedback_submitted': undefined,
+ 'call.user_muted': undefined,
'connection.error': undefined,Also applies to: 474-484
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/client/src/store/CallState.ts` around lines 343 - 344, Add the
missing call.user_muted property to the eventHandlers initializer in CallState,
using an explicit undefined handler or the intended state-update callback so it
satisfies the required WSEvent mapped type. Preserve all existing event
handlers.
Bundle sizeBuilt package output. Sizes in KB; delta vs
|
1 participant
💡 Overview
📝 Implementation notes
🎫 Ticket: https://linear.app/stream/issue/XYZ-123
📑 Docs: https://github.com/GetStream/docs-content/pull/
Summary by CodeRabbit
New Features
Bug Fixes