Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions transports/daily/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ All notable changes to **Pipecat Daily WebRTC Transport** will be documented in
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

- Added error handling around `daily.sendAppMessage`
- Set `_maxMessageSize` to the underlyling daily instance's max size (defaults to 10MB)

## [1.6.0]

- Bump client-js version to work with latest 1.6.0 and support latest features
Expand Down
18 changes: 17 additions & 1 deletion transports/daily/src/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ import Daily, {
DailyEventObjectTrack,
DailyFactoryOptions,
DailyParticipant,
DailyRoomInfo,
} from "@daily-co/daily-js";
import {
DeviceArray,
DeviceError,
MessageTooLargeError,
Participant,
PipecatClientOptions,
RTVIError,
Expand Down Expand Up @@ -482,6 +484,10 @@ export class DailyTransport extends Transport {

if (this._abortController?.signal.aborted) return;

const r = await this._daily.room();
this._maxMessageSize =
(r as DailyRoomInfo)?.domainConfig?.max_app_message_size ||
10 * 1024 * 1024;
this.state = "connected";

this._callbacks.onConnected?.();
Expand Down Expand Up @@ -597,7 +603,17 @@ export class DailyTransport extends Transport {
}

public sendMessage(message: RTVIMessage) {
this._daily.sendAppMessage(message, "*");
try {
this._daily.sendAppMessage(message, "*");
} catch (error: unknown) {
if (
error instanceof Error &&
error.message.includes("Message data too large")
) {
throw new MessageTooLargeError(error.message);
}
throw error;
}
}

private handleAppMessage(ev: DailyEventObjectAppMessage) {
Expand Down
4 changes: 4 additions & 0 deletions transports/openai-realtime-webrtc-transport/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to **Pipecat OpenAIRealTimeWebRTCTransport** will be documen
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

- Added default case to `sendMessage` to properly throw an `UnsupportedFeatureError`

## [1.5.0]

- Bump client-js version to work with latest 1.6.0 and support latest features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Transport,
TransportStartError,
TransportState,
UnsupportedFeatureError,
logger,
} from "@pipecat-ai/client-js";

Expand Down Expand Up @@ -388,6 +389,11 @@ export class OpenAIRealTimeWebRTCTransport extends Transport {
);
break;
}
default:
throw new UnsupportedFeatureError(
message.type,
"OpenAIRealTimeWebRTCTransport",
);
}
}

Expand Down
6 changes: 6 additions & 0 deletions transports/small-webrtc-transport/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to **Pipecat Small WebRTC Transport** will be documented in
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

- Added message size checking to `sendMessage()` to ensure no large
messages are sent, causing the data channel to fail and not recover.
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I haven’t tested this yet. Is this what happens if we try to send a message that’s too large, the data channel never recover ?
I would expect to just receive an error and for it to keep working afterward. 🤕

- Set `_maxMessageSize` to the peer connections's max size (defaults to 64KB)

## [1.9.0]

### Added
Expand Down
14 changes: 12 additions & 2 deletions transports/small-webrtc-transport/src/smallWebRTCTransport.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import cloneDeep from "lodash/cloneDeep";
import {
APIRequest,
isAPIRequest,
logger,
makeRequest,
messageSizeWithinLimit,
MessageTooLargeError,
RTVIError,
RTVIMessage,
PipecatClientOptions,
Expand All @@ -10,8 +14,6 @@ import {
TransportStartError,
TransportState,
UnsupportedFeatureError,
APIRequest,
isAPIRequest,
} from "@pipecat-ai/client-js";
import { MediaManager } from "../../../lib/media-mgmt/mediaManager";
import { DailyMediaManager } from "../../../lib/media-mgmt/dailyMediaManager";
Expand Down Expand Up @@ -428,6 +430,13 @@ export class SmallWebRTCTransport extends Transport {
logger.warn(`Datachannel is not ready. Message not sent: ${message}`);
return;
}

// Note: This shouldn't happen since client-js should have already checked this
if (!messageSizeWithinLimit(message, this._maxMessageSize)) {
throw new MessageTooLargeError(
"Message data too large. Max size is " + this._maxMessageSize,
);
}
this.dc?.send(JSON.stringify(message));
}

Expand Down Expand Up @@ -903,6 +912,7 @@ export class SmallWebRTCTransport extends Transport {

dc.addEventListener("open", () => {
logger.debug("datachannel opened");
this._maxMessageSize = this.pc?.sctp?.maxMessageSize ?? 64 * 1024;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was inspecting this and noticed that Chrome requested (offer SDP) a max message size of:
a=max-message-size:262144

However, the answer (SDP) from aiortc was:
a=max-message-size:65536

So the default value we’re using is correct.

However, if we want to be 100% accurate, we could retrieve this exact value from the SDP answer, in case aiortc increases this limit in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per our discussion, this is happening with the above this.pc?.sctp?.maxMessageSize

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I have missed this one. 🙂

if (this._connectResolved) {
this.syncTrackStatus();
this._connectResolved();
Expand Down
4 changes: 4 additions & 0 deletions transports/websocket-transport/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to **Pipecat Websocket Transport** will be documented in thi
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

- Set `_maxMessageSize` to the server's supported max size (1 MB)

## [1.6.0]

- Bump client-js version to work with latest 1.6.0 and support latest features
Expand Down
1 change: 1 addition & 0 deletions transports/websocket-transport/src/webSocketTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export class WebSocketTransport extends Transport {
);
this._ws = null;
this._serializer = opts.serializer || new ProtobufFrameSerializer();
this._maxMessageSize = 1024 * 1024; // python websockets default to 1MB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to allow users to change this default value when creating the transport ? I’m not sure whether they can change it on the server side if they have a custom WebSocket implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per our discussion, we think this is fine for now.

}

initialize(
Expand Down