feat: added call-sdk package #187
Conversation
This commit adds the call-sdk package, which provides the necessary components for building a video calling SDK using MediaSoup. It includes: - Initial package.json with dependencies for mediasoup-client, react, and react-dom. - src/index.ts: Exports core classes, hooks, types, and error handling. - src/providers/CallProvider.tsx: React context provider for managing call state. - src/hooks/useCall.ts: React hook for accessing call functionality. - src/hooks/useMediaDevices.ts: React hook for managing media devices. - src/services/call-client.ts: The main CallClient class orchestrating the call functionality. - src/services/mediasoup-service.ts: Service for handling MediaSoup interactions. - src/services/signaling-client.ts: Service for WebSocket communication with the signaling server. - src/services/media-manager.ts: Service for managing media devices and streams. - src/types/call.ts: Type definitions for call-related data. - src/types/participant.ts: Type definitions for participants. - src/utils/error-handler.ts: Error handling utilities.
|
@SyedMuzamilM is attempting to deploy a commit to the cloudec31-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces the initial implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant CallProvider
participant CallClient
participant MediasoupService
participant MediaManager
participant SignalingClient
participant PeerManager
App->>CallProvider: Render with signalingUrl
CallProvider->>CallClient: Instantiate
CallClient->>SignalingClient: Connect to signaling server
CallClient->>MediasoupService: Setup media transports
CallClient->>MediaManager: Get local media streams
CallClient->>PeerManager: Track participants
App->>CallProvider: Use hooks (e.g., useCall)
CallProvider->>CallClient: Join/leave call, toggle media, etc.
CallClient->>MediasoupService: Produce/consume media
CallClient->>SignalingClient: Signal events (join, leave, etc.)
CallClient->>PeerManager: Update participant state
CallProvider->>App: Expose call state and actions via context/hooks
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (8)
packages/call-sdk/src/hooks/useMediaDevices.ts (1)
95-105: Consider removing callbacks from dependency arrayIncluding
checkPermissionsandupdateDevicesin the dependency array may cause this effect to re-run unnecessarily since these are defined withuseCallback.Since these callbacks have no dependencies (empty arrays), you can safely remove them from the dependency array:
- }, [checkPermissions, updateDevices]); + }, []); // eslint-disable-line react-hooks/exhaustive-depspackages/call-sdk/src/services/signaling-client.ts (2)
134-137: Consider configurable timeout for different request typesThe current implementation uses a fixed 10-second timeout for all requests. Some operations (like large file transfers or slow network conditions) might need longer timeouts.
Add an optional timeout parameter:
- async sendRequest<T = any>(type: string, data: any = {}): Promise<T> { + async sendRequest<T = any>(type: string, data: any = {}, timeout = 10000): Promise<T> { if (!this.ws || this.ws.readyState !== WebSocket.OPEN) { throw new Error("WebSocket not connected"); } const reqId = this.generateRequestId(); const message = { type, reqId, ...data }; return new Promise<T>((resolve, reject) => { const timeoutId = setTimeout(() => { this.pendingRequests.delete(reqId); reject(new Error(`Request timeout for ${type}`)); - }, 10000); + }, timeout);
311-313: Improve request ID generation for better uniquenessUsing Math.random() for ID generation could theoretically result in collisions, especially under high load.
Consider using a more robust ID generation method:
private generateRequestId(): string { - return Math.random().toString(36).substring(2, 15); + return `${Date.now()}-${Math.random().toString(36).substring(2, 15)}`; }Or use a UUID library if available in the project.
packages/call-sdk/src/services/mediasoup-service.ts (2)
115-125: Consider propagating consumer errors instead of just loggingWhen consuming existing producers fails, the error is only logged with console.warn. This might hide important issues from the application.
Consider collecting and reporting failed consumers:
+ const failedConsumers: Array<{ id: string; error: any }> = []; for (const producer of joinResponse.producers) { try { await this.consumeProducer(producer.id); } catch (error) { console.warn( "Failed to consume existing producer:", producer.id, error ); + failedConsumers.push({ id: producer.id, error }); } } + + if (failedConsumers.length > 0) { + this.emit("error", new Error(`Failed to consume ${failedConsumers.length} existing producers`)); + }
424-434: Optimize consumer lookup performanceThe current implementation searches through all consumers to find one by producer ID, which is O(n) complexity.
Consider maintaining a producer-to-consumer mapping for O(1) lookup:
private consumers = new Map<string, any>(); +private producerToConsumer = new Map<string, string>(); // producerId -> consumerId // In consumeProducer: this.consumers.set(consumer.id, consumer); +this.producerToConsumer.set(consumeResponse.producerId, consumer.id); // In handleProducerClosed: private handleProducerClosed(data: any): void { - const consumer = Array.from(this.consumers.values()).find( - (c) => (c as any).producerId === data.producerId - ); + const consumerId = this.producerToConsumer.get(data.producerId); + const consumer = consumerId ? this.consumers.get(consumerId) : undefined; if (consumer) { consumer.close(); this.consumers.delete(consumer.id); + this.producerToConsumer.delete(data.producerId); this.emit("consumerClosed", consumer.id); } }packages/call-sdk/src/services/peer-manager.ts (1)
188-191: Consider making audio level threshold configurableThe speaking threshold of 0.1 is hardcoded. This value might need adjustment based on the audio processing pipeline and normalization used.
Make the threshold configurable:
+ private speakingThreshold = 0.1; + + /** + * Set the audio level threshold for considering someone as speaking + */ + setSpeakingThreshold(threshold: number): void { + this.speakingThreshold = threshold; + } + /** * Check if a participant is currently speaking (has high audio level) */ isParticipantSpeaking(participantId: string): boolean { const level = this.audioLevels.get(participantId) || 0; - return level > 0.1; // Threshold for considering someone as speaking + return level > this.speakingThreshold; }Also update the threshold usage in
updateDominantSpeakerFromAudioLevelsandgetSortedParticipants.packages/call-sdk/src/services/media-manager.ts (1)
13-27: Consider using typed EventEmitter for better type safety.The current interface/class merging pattern is flagged by static analysis as potentially unsafe.
Consider using a typed EventEmitter implementation or composition instead of declaration merging:
import { TypedEmitter } from 'tiny-typed-emitter'; interface MediaManagerEvents { devicesChanged: (devices: MediaDeviceInfo[]) => void; streamCreated: (stream: MediaStream, source: ProducerSource) => void; streamEnded: (source: ProducerSource) => void; error: (error: Error) => void; } export class MediaManager extends TypedEmitter<MediaManagerEvents> { // ... rest of the implementation }This provides better type safety and avoids the declaration merging issue.
packages/call-sdk/src/types/call.ts (1)
207-211: Improve ServerMessage type safety.The
[key: string]: anyindex signature inServerMessagereduces type safety and allows any property with any type.Consider using a discriminated union instead of an index signature:
- export interface ServerMessage { - type: string; - reqId?: string; - [key: string]: any; - } + export interface BaseServerMessage { + type: string; + reqId?: string; + } + + export type ServerMessage = + | JoinRoomRequest + | JoinRoomResponse + | CreateTransportRequest + | CreateTransportResponse + // ... other message types + | PeerLeftNotification;This provides better type safety and IntelliSense support while maintaining flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/call-sdk/package.json(1 hunks)packages/call-sdk/src/hooks/useCall.ts(1 hunks)packages/call-sdk/src/hooks/useMediaDevices.ts(1 hunks)packages/call-sdk/src/index.ts(1 hunks)packages/call-sdk/src/providers/CallProvider.tsx(1 hunks)packages/call-sdk/src/services/call-client.ts(1 hunks)packages/call-sdk/src/services/media-manager.ts(1 hunks)packages/call-sdk/src/services/mediasoup-service.ts(1 hunks)packages/call-sdk/src/services/peer-manager.ts(1 hunks)packages/call-sdk/src/services/signaling-client.ts(1 hunks)packages/call-sdk/src/types/call.ts(1 hunks)packages/call-sdk/src/types/participant.ts(1 hunks)packages/call-sdk/src/utils/error-handler.ts(1 hunks)packages/call-sdk/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/call-sdk/src/services/media-manager.ts (3)
packages/call-sdk/src/index.ts (2)
ProducerSource(22-22)MediaManager(5-5)packages/call-sdk/src/types/call.ts (1)
ProducerSource(15-15)apps/web/hooks/useMediasoupClient.ts (1)
Producer(22-29)
packages/call-sdk/src/utils/error-handler.ts (1)
packages/call-sdk/src/index.ts (8)
CallErrorType(38-38)CallErrorDetails(45-45)CallError(37-37)ErrorHandler(39-39)errorHandler(40-40)createCallError(41-41)handleError(42-42)onError(43-43)
🪛 Biome (2.1.2)
packages/call-sdk/src/services/mediasoup-service.ts
[error] 36-36: This class is unsafely merged with an interface.
The interface is declared here.
The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
(lint/suspicious/noUnsafeDeclarationMerging)
packages/call-sdk/src/services/media-manager.ts
[error] 27-27: This class is unsafely merged with an interface.
The interface is declared here.
The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
(lint/suspicious/noUnsafeDeclarationMerging)
packages/call-sdk/src/services/call-client.ts
[error] 41-41: This class is unsafely merged with an interface.
The interface is declared here.
The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
(lint/suspicious/noUnsafeDeclarationMerging)
packages/call-sdk/src/services/signaling-client.ts
[error] 45-45: This class is unsafely merged with an interface.
The interface is declared here.
The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
(lint/suspicious/noUnsafeDeclarationMerging)
🔇 Additional comments (6)
packages/call-sdk/tsconfig.json (1)
1-14: LGTM! Well-configured TypeScript setup.The configuration appropriately extends the base React library config, uses modern ES2022 target, and sets up clean path aliases for internal imports. The exclusions and module settings are standard for a publishable library.
packages/call-sdk/src/hooks/useCall.ts (1)
56-151: LGTM! Well-structured React hook with good patterns.The hook follows React best practices:
- Proper use of useCallback for memoization
- Clean derived state calculations
- Good separation of concerns in the return object
- Appropriate effect for reconnection handling
The participant count calculation correctly includes self, and the state derivation is efficient.
packages/call-sdk/src/index.ts (1)
1-45: LGTM! Well-organized export structure.The exports are logically grouped with clear comments, providing a comprehensive API surface for the SDK. The separation between core services, React components, types, and error handling makes the SDK easy to consume.
packages/call-sdk/src/types/participant.ts (1)
1-135: LGTM! Excellent type definitions with comprehensive coverage.The participant types are well-designed with:
- Clear interface hierarchy using extension
- Proper discrimination between local and remote participants
- Type guard functions for runtime type safety
- Comprehensive JSDoc documentation
- Good separation of concerns between media state and participant metadata
The type system provides strong foundations for the SDK's participant management.
packages/call-sdk/src/providers/CallProvider.tsx (1)
98-199: Memory leak: CallClient not destroyed when signalingUrl changesThe effect creates a new CallClient when signalingUrl changes but doesn't destroy the previous instance. This will cause memory leaks and duplicate event listeners.
Add cleanup logic to destroy the previous CallClient:
useEffect(() => { + const currentClient = callClientRef.current; if (!callClientRef.current) { callClientRef.current = new CallClient(signalingUrl); // ... rest of initialization } return () => { + if (currentClient && currentClient !== callClientRef.current) { + currentClient.destroy(); + } navigator.mediaDevices?.removeEventListener( "devicechange", updateDevices ); }; }, [signalingUrl]);Alternatively, consider not recreating the CallClient when signalingUrl changes, or destroy and recreate it properly.
Likely an incorrect or invalid review comment.
packages/call-sdk/src/utils/error-handler.ts (1)
296-353: Add validation for recovery functions.The recovery execution assumes specific functions exist in the error context but only checks if they're functions, not if they're the expected recovery functions.
Add proper validation and type safety:
private async executeRecovery(error: CallError): Promise<void> { + // Define recovery context interface + interface RecoveryContext { + reconnect?: () => Promise<void>; + recreateTransport?: () => Promise<void>; + recreateProducer?: () => Promise<void>; + recreateConsumer?: () => Promise<void>; + reinitializeMedia?: () => Promise<void>; + } + + const context = error.context as RecoveryContext | undefined; + if (!context) { + console.warn(`No recovery context for error type: ${error.type}`); + return; + } switch (error.type) { case CallErrorType.CONNECTION_LOST: case CallErrorType.WEBSOCKET_ERROR: - if ( - error.context?.reconnect && - typeof error.context.reconnect === "function" - ) { - await error.context.reconnect(); + if (context.reconnect && typeof context.reconnect === "function") { + await context.reconnect(); } break; // Apply similar changes to other cases... } }Likely an incorrect or invalid review comment.
| "dependencies": { | ||
| "mediasoup-client": "^3.14.0", | ||
| "react": "^19.1.1", | ||
| "react-dom": "^19.1.1" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review React version consistency.
There's a potential version conflict: the dependencies specify React ^19.1.1 but peerDependencies allow >=18.0.0. This could cause issues for consumers using React 18.
Consider moving React to peerDependencies only or align the versions:
"dependencies": {
- "mediasoup-client": "^3.14.0",
- "react": "^19.1.1",
- "react-dom": "^19.1.1"
+ "mediasoup-client": "^3.14.0"
},📝 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.
| "dependencies": { | |
| "mediasoup-client": "^3.14.0", | |
| "react": "^19.1.1", | |
| "react-dom": "^19.1.1" | |
| }, | |
| "dependencies": { | |
| "mediasoup-client": "^3.14.0" | |
| }, |
🤖 Prompt for AI Agents
In packages/call-sdk/package.json around lines 27 to 31, the React version in
dependencies (^19.1.1) conflicts with the peerDependencies allowing >=18.0.0,
which may cause version mismatches for consumers. To fix this, remove React and
react-dom from dependencies and instead specify them only in peerDependencies
with a consistent version range that includes both React 18 and 19, ensuring
compatibility and avoiding duplicate React installations.
| "author": "Call0", | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "mediasoup-client": "^3.14.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify mediasoup-client version compatibility.
Ensure that mediasoup-client version 3.14.0 is compatible with the latest WebRTC standards and doesn't have known security vulnerabilities.
🌐 Web query:
What is the latest version of mediasoup-client and are there any security advisories for version 3.14.0?
💡 Result:
The latest version of mediasoup-client as of August 2025 is 3.18.0[10]. For mediasoup specifically (the server component), the most recent version is 3.16.6, according to the Snyk vulnerability database, but mediasoup-client 3.18.0 was released July 31, 2025[10][4].
Regarding security advisories for mediasoup-client version 3.14.0, there are no public reports of direct security vulnerabilities specifically affecting mediasoup-client 3.14.0 in current vulnerability databases as of August 2025[4]. The Snyk advisories page for mediasoup lists recent versions but does not show unresolved vulnerabilities for 3.14.0[4]. However, as mediasoup and mediasoup-client are tightly coordinated projects, and the changelog documents ongoing bug fixes and improvements, it is recommended you check the official mediasoup GitHub and relevant security trackers for any targeted advisories before deploying or maintaining version 3.14.0[2].
In summary:
- Latest mediasoup-client: 3.18.0 (released July 31, 2025)[10].
- No public, unresolved security advisories specifically for mediasoup-client 3.14.0[4].
- Upgrading to the latest stable release is recommended, as upstream updates sometimes include security-related protocol and compatibility fixes[2][10].
Citations:
- 1: https://mediasoup.discourse.group/t/mediasoup-client-3-10-0-released-with-new-and-better-browser-detection/6663
- 2: https://github.com/versatica/mediasoup/blob/v3/CHANGELOG.md
- 3: https://mediasoup.org/documentation/v3/mediasoup-client/api/
- 4: https://security.snyk.io/package/npm/mediasoup
- 5: https://mvnrepository.com/artifact/org.mediasoup.droid/mediasoup-client
- 6: https://docs.l7mp.io/en/v0.21.0/examples/mediasoup/
- 7: https://www.npmjs.com/package/mediasoup?activeTab=versions
- 8: https://classic.yarnpkg.com/en/package/@mafalda-sfu/remote-mediasoup-client-mock
- 9: https://github.com/versatica/mediasoup-client-aiortc
- 10: https://mediasoup.discourse.group/t/mediasoup-3-18-0-released/6807
Upgrade mediasoup-client to the latest stable release (3.18.0)
Although there are no public security advisories for 3.14.0, the latest v3.18.0 (released July 31, 2025) includes compatibility and bug-fix updates against evolving WebRTC standards. Please update the dependency to benefit from these upstream improvements.
• packages/call-sdk/package.json (line 28)
- Change
"mediasoup-client": "^3.14.0", - To
"mediasoup-client": "^3.18.0",
🤖 Prompt for AI Agents
In packages/call-sdk/package.json at line 28, update the mediasoup-client
dependency version from "^3.14.0" to "^3.18.0" to use the latest stable release
with improved compatibility and bug fixes. Simply replace the version string to
"^3.18.0" and save the file.
| stats?: CallStats; | ||
|
|
||
| // Advanced | ||
| callClient: any; // CallClient instance for advanced usage |
There was a problem hiding this comment.
Replace 'any' type with proper CallClient type.
Using any breaks type safety and defeats the purpose of TypeScript. Import and use the actual CallClient type.
+import type { CallClient } from '../services/call-client';
// Advanced
- callClient: any; // CallClient instance for advanced usage
+ callClient: CallClient; // CallClient instance for advanced usage📝 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.
| callClient: any; // CallClient instance for advanced usage | |
| import type { CallClient } from '../services/call-client'; | |
| // Advanced | |
| callClient: CallClient; // CallClient instance for advanced usage |
🤖 Prompt for AI Agents
In packages/call-sdk/src/hooks/useCall.ts at line 49, the callClient variable is
typed as 'any', which breaks type safety. Import the CallClient type from its
module and replace 'any' with this proper type annotation to ensure type safety
and better code clarity.
| const permissionError = error as Error; | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| hasAudioPermission: !permissionError.message.includes("audio"), | ||
| hasVideoPermission: !permissionError.message.includes("video"), | ||
| })); |
There was a problem hiding this comment.
Unreliable permission detection logic
The current implementation assumes permissions are granted based on whether the error message contains "audio" or "video". This is browser-dependent and unreliable.
Consider a more robust approach:
} catch (error) {
- const permissionError = error as Error;
- setState((prev) => ({
- ...prev,
- hasAudioPermission: !permissionError.message.includes("audio"),
- hasVideoPermission: !permissionError.message.includes("video"),
- }));
+ // Try each permission separately to determine which failed
+ let hasAudioPermission = false;
+ let hasVideoPermission = false;
+
+ try {
+ const audioStream = await navigator.mediaDevices.getUserMedia({ audio: true });
+ audioStream.getTracks().forEach((track) => track.stop());
+ hasAudioPermission = true;
+ } catch {}
+
+ try {
+ const videoStream = await navigator.mediaDevices.getUserMedia({ video: true });
+ videoStream.getTracks().forEach((track) => track.stop());
+ hasVideoPermission = true;
+ } catch {}
+
+ setState((prev) => ({
+ ...prev,
+ hasAudioPermission,
+ hasVideoPermission,
+ }));
}🤖 Prompt for AI Agents
In packages/call-sdk/src/hooks/useMediaDevices.ts around lines 86 to 91, the
current logic checks permission by inspecting error message strings for "audio"
or "video", which is unreliable and browser-dependent. Replace this with a more
robust method by checking the error's name or code properties against known
permission error identifiers, or use the Permissions API if available, to
accurately determine audio and video permission statuses before updating the
state.
| // Core provider and hooks | ||
| export { CallProvider } from "./providers/CallProvider"; | ||
| export { useCall } from "./hooks/useCall"; | ||
| export { useParticipants } from "./hooks/useParticipants"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify useParticipants hook implementation.
The useParticipants hook is exported but its implementation file wasn't provided in the review. Ensure this hook exists and is properly implemented.
🏁 Script executed:
#!/bin/bash
# Description: Check if useParticipants hook exists and verify its implementation
fd -t f "useParticipants" packages/call-sdk/src/Length of output: 48
🏁 Script executed:
#!/bin/bash
# 1. List all hook files under packages/call-sdk/src/hooks
echo "=== Hook files in packages/call-sdk/src/hooks ==="
fd -t f -e ts -e tsx packages/call-sdk/src/hooks
# 2. Search for any references to useParticipants across the call-sdk source
echo -e "\n=== Occurrences of 'useParticipants' in call-sdk ==="
rg -n "useParticipants" -C 2 packages/call-sdk/srcLength of output: 1139
🏁 Script executed:
#!/bin/bash
# 1. List all files in the hooks directory
echo "=== Hook files under packages/call-sdk/src/hooks ==="
fd . packages/call-sdk/src/hooks
# 2. Specifically look for any file named useParticipants.*
echo -e "\n=== Files matching 'useParticipants' under hooks ==="
fd -t f useParticipants packages/call-sdk/src/hooksLength of output: 416
Missing useParticipants Hook Implementation
The export in packages/call-sdk/src/index.ts line 11 references ./hooks/useParticipants, but no corresponding file exists in packages/call-sdk/src/hooks. This will break the build.
• Location to fix:
- packages/call-sdk/src/index.ts:11
• Recommended actions:
- Create and implement
packages/call-sdk/src/hooks/useParticipants.ts(exporting theuseParticipantshook). - Or, if the hook is not needed, remove the export from the index file.
🤖 Prompt for AI Agents
In packages/call-sdk/src/index.ts at line 11, the export statement references a
useParticipants hook from ./hooks/useParticipants, but the corresponding file
does not exist, causing build failures. To fix this, either create and implement
the useParticipants hook in packages/call-sdk/src/hooks/useParticipants.ts
exporting the hook properly, or if the hook is not required, remove the export
statement from the index.ts file.
| navigator.mediaDevices.addEventListener("devicechange", () => { | ||
| this.updateDeviceList(); | ||
| }); |
There was a problem hiding this comment.
Fix memory leak by cleaning up event listeners.
The devicechange event listener is never removed, causing a memory leak.
Apply this diff to properly clean up:
+ private deviceChangeHandler = () => {
+ this.updateDeviceList();
+ };
private async initializeDeviceMonitoring(): Promise<void> {
// ... existing code ...
// Monitor device changes
- navigator.mediaDevices.addEventListener("devicechange", () => {
- this.updateDeviceList();
- });
+ navigator.mediaDevices.addEventListener("devicechange", this.deviceChangeHandler);
}
/**
* Cleanup resources
*/
destroy(): void {
this.stopAllStreams();
+ navigator.mediaDevices.removeEventListener("devicechange", this.deviceChangeHandler);
this.removeAllListeners();
}Also applies to: 396-400
🤖 Prompt for AI Agents
In packages/call-sdk/src/services/media-manager.ts around lines 55 to 57 and
also lines 396 to 400, the 'devicechange' event listener added to
navigator.mediaDevices is never removed, causing a memory leak. To fix this,
store the event listener callback as a named function or variable and add a
method to remove this event listener using
navigator.mediaDevices.removeEventListener when the component or service is
disposed or no longer needed. This ensures proper cleanup of event listeners and
prevents memory leaks.
| if (!this.selectedDevices.audioInput && this.audioDevices.length > 0) { | ||
| this.selectedDevices.audioInput = this.audioDevices[0]!.deviceId; | ||
| } | ||
| if (!this.selectedDevices.videoInput && this.videoDevices.length > 0) { | ||
| this.selectedDevices.videoInput = this.videoDevices[0]!.deviceId; | ||
| } | ||
| if ( | ||
| !this.selectedDevices.audioOutput && | ||
| this.audioOutputDevices.length > 0 | ||
| ) { | ||
| this.selectedDevices.audioOutput = this.audioOutputDevices[0]!.deviceId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid non-null assertions on array access.
Using non-null assertions on array access is risky and bypasses TypeScript's safety checks.
Use safer array access patterns:
- if (!this.selectedDevices.audioInput && this.audioDevices.length > 0) {
- this.selectedDevices.audioInput = this.audioDevices[0]!.deviceId;
- }
+ if (!this.selectedDevices.audioInput && this.audioDevices.length > 0) {
+ const firstDevice = this.audioDevices[0];
+ if (firstDevice) {
+ this.selectedDevices.audioInput = firstDevice.deviceId;
+ }
+ }Or use optional chaining:
this.selectedDevices.audioInput = this.audioDevices[0]?.deviceId || "";🤖 Prompt for AI Agents
In packages/call-sdk/src/services/media-manager.ts around lines 85 to 96, the
code uses non-null assertions (!) when accessing the first element of device
arrays, which bypasses TypeScript safety checks. Replace these assertions by
using optional chaining and fallback values, for example, assign
this.selectedDevices.audioInput to this.audioDevices[0]?.deviceId or an empty
string if undefined. Apply this pattern consistently for audioInput, videoInput,
and audioOutput device assignments to ensure safer array access.
| const producers: any[] = []; | ||
|
|
||
| for (const track of stream.getTracks()) { | ||
| try { | ||
| const producer = await this.sendTransport.produce({ | ||
| track, | ||
| encodings: this.getEncodings(track.kind as "audio" | "video"), | ||
| codecOptions: this.getCodecOptions(track.kind as "audio" | "video"), | ||
| }); | ||
|
|
||
| // Store producer | ||
| this.producers.set(source, producer); | ||
|
|
||
| // Handle producer events | ||
| producer.on("transportclose", () => { | ||
| this.producers.delete(source); | ||
| this.emit("producerClosed", producer.id, source); | ||
| }); | ||
|
|
||
| producer.on("trackended", () => { | ||
| this.closeProducer(source); | ||
| }); | ||
|
|
||
| producers.push(producer); | ||
| this.emit("producerCreated", producer, source); | ||
|
|
||
| console.log( | ||
| `Producer created: ${producer.id}, kind: ${track.kind}, source: ${source}` | ||
| ); | ||
| } catch (error) { | ||
| console.error(`Failed to produce ${track.kind} track:`, error); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| return producers; |
There was a problem hiding this comment.
Producer storage overwrites previous producers for the same source
The code creates producers for each track but stores them in a map keyed by source. This means if a stream has both audio and video tracks, only the last producer will be stored in the map.
Modify the producer storage to handle multiple tracks per source:
- private producers = new Map<ProducerSource, any>();
+ private producers = new Map<string, any>(); // key: "source:kind"
// In produceMedia:
- this.producers.set(source, producer);
+ const producerKey = `${source}:${track.kind}`;
+ this.producers.set(producerKey, producer);
// Update other methods accordingly:
async closeProducer(source: ProducerSource): Promise<void> {
- const producer = this.producers.get(source);
+ // Close all producers for this source
+ const producersToClose = Array.from(this.producers.entries())
+ .filter(([key]) => key.startsWith(`${source}:`));
+
+ for (const [key, producer] of producersToClose) {
if (producer) {
try {
await this.signalingClient.closeProducer(producer.id);
producer.close();
- this.producers.delete(source);
+ this.producers.delete(key);
this.emit("producerClosed", producer.id, source);
} catch (error) {
console.error("Failed to close producer:", error);
throw error;
}
}
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/call-sdk/src/services/mediasoup-service.ts between lines 180 and
215, the current code stores only one producer per source in the producers map,
overwriting previous producers if multiple tracks exist for the same source. To
fix this, change the map value to hold an array of producers per source. When
adding a new producer, push it into the array for that source instead of
replacing the existing entry. Also update the event handlers and any related
logic to handle multiple producers per source accordingly.
| private categorizeError(error: Error): CallErrorType { | ||
| const message = error.message.toLowerCase(); | ||
|
|
||
| // Connection errors | ||
| if (message.includes("websocket") || message.includes("connection")) { | ||
| if (message.includes("failed") || message.includes("refused")) { | ||
| return CallErrorType.CONNECTION_FAILED; | ||
| } | ||
| if (message.includes("lost") || message.includes("closed")) { | ||
| return CallErrorType.CONNECTION_LOST; | ||
| } | ||
| return CallErrorType.WEBSOCKET_ERROR; | ||
| } | ||
|
|
||
| // Media errors | ||
| if (message.includes("permission") || message.includes("denied")) { | ||
| return CallErrorType.MEDIA_PERMISSION_DENIED; | ||
| } | ||
| if ( | ||
| message.includes("device") || | ||
| message.includes("microphone") || | ||
| message.includes("camera") | ||
| ) { | ||
| return CallErrorType.MEDIA_DEVICE_ERROR; | ||
| } | ||
| if (message.includes("screen") || message.includes("display")) { | ||
| return CallErrorType.SCREEN_SHARE_ERROR; | ||
| } | ||
| if (message.includes("stream") || message.includes("track")) { | ||
| return CallErrorType.MEDIA_STREAM_ERROR; | ||
| } | ||
|
|
||
| // MediaSoup errors | ||
| if (message.includes("transport")) { | ||
| return CallErrorType.TRANSPORT_ERROR; | ||
| } | ||
| if (message.includes("producer")) { | ||
| return CallErrorType.PRODUCER_ERROR; | ||
| } | ||
| if (message.includes("consumer")) { | ||
| return CallErrorType.CONSUMER_ERROR; | ||
| } | ||
| if (message.includes("rtp") || message.includes("capabilities")) { | ||
| return CallErrorType.RTP_CAPABILITIES_ERROR; | ||
| } | ||
|
|
||
| // Room errors | ||
| if (message.includes("room not found") || message.includes("404")) { | ||
| return CallErrorType.ROOM_NOT_FOUND; | ||
| } | ||
| if (message.includes("room full") || message.includes("capacity")) { | ||
| return CallErrorType.ROOM_FULL; | ||
| } | ||
| if (message.includes("participant limit")) { | ||
| return CallErrorType.PARTICIPANT_LIMIT_EXCEEDED; | ||
| } | ||
|
|
||
| // Auth errors | ||
| if (message.includes("unauthorized") || message.includes("401")) { | ||
| return CallErrorType.UNAUTHORIZED; | ||
| } | ||
| if (message.includes("token") || message.includes("expired")) { | ||
| return CallErrorType.TOKEN_EXPIRED; | ||
| } | ||
|
|
||
| // Network errors | ||
| if (message.includes("network") || message.includes("timeout")) { | ||
| return CallErrorType.NETWORK_ERROR; | ||
| } | ||
|
|
||
| return CallErrorType.UNKNOWN_ERROR; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error categorization robustness.
The current error categorization relies on simple string matching which is fragile and prone to false positives. For example, "401" could appear in non-authentication contexts, and error messages might change across library versions.
Consider using a more robust approach:
- Use error codes or structured error types from the underlying libraries
- Check error properties beyond just the message
- Add a mapping configuration for known error patterns
Example improvement:
private categorizeError(error: Error): CallErrorType {
// Check for specific error types first
if (error.name === 'NotAllowedError' || error.name === 'NotFoundError') {
return CallErrorType.MEDIA_PERMISSION_DENIED;
}
// Check for error codes if available
if ('code' in error) {
const errorCode = (error as any).code;
switch (errorCode) {
case 'ECONNREFUSED':
return CallErrorType.CONNECTION_FAILED;
// Add more error codes...
}
}
// Fall back to message matching as last resort
const message = error.message.toLowerCase();
// ... existing logic
}🤖 Prompt for AI Agents
In packages/call-sdk/src/utils/error-handler.ts between lines 157 and 228,
improve the categorizeError method by first checking error.name and error.code
properties for known error types and codes from underlying libraries to avoid
fragile string matching. Implement a mapping configuration for error codes to
CallErrorType values, and only use message string matching as a fallback. This
will make error categorization more robust and reduce false positives.
| retryable?: boolean; | ||
| } | ||
| ): CallError { | ||
| const { recoverable, retryable } = errorHandler["getErrorProperties"](type); |
There was a problem hiding this comment.
Fix access to private method.
The code is accessing a private method getErrorProperties using array notation, which bypasses TypeScript's access modifiers. This is a bad practice that breaks encapsulation.
Apply this diff to properly handle the access:
- const { recoverable, retryable } = errorHandler["getErrorProperties"](type);
+ // Make getErrorProperties public or use a different approach
+ const { recoverable, retryable } = CallError.getDefaultProperties(type);You'll need to either:
- Make
getErrorPropertiesa public method, or - Create a static method on
CallErrorclass for getting default properties
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/call-sdk/src/utils/error-handler.ts at line 392, the code accesses
the private method getErrorProperties using array notation, which breaks
encapsulation. To fix this, either change the getErrorProperties method's
visibility to public so it can be accessed directly, or implement a new static
method on the CallError class that returns the default error properties and use
that method instead.
1 participant
Description
This commit adds the call-sdk package, which provides the necessary components for building a video calling SDK using MediaSoup. It includes:
Checklist
pnpm buildorpnpm lintwith no errorsSummary by CodeRabbit
New Features
Chores