Skip to content

Commit

Permalink
media: catch Type errors when adding Transceiver in Safari17Handler
Browse files Browse the repository at this point in the history
We're seeing this error when running this handler in the Webkit build
used by Playwright in the e2e test suite in Github Actions

I can't reproduce it locally, and 'normal' builds of safari work just
fine, so this is a compromise to get the tests working
  • Loading branch information
kevinwhereby committed Dec 6, 2024
1 parent 7ed6942 commit 8093032
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 10 deletions.
15 changes: 14 additions & 1 deletion apps/sample-app/test/testsuites/video.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, expect } from "@playwright/test";
import { test, expect, ConsoleMessage } from "@playwright/test";
import { createTransientRoom, deleteTransientRoom, joinRoom, RoomMode } from "./utils/room";
import {
countFrames,
Expand Down Expand Up @@ -36,6 +36,19 @@ roomModes.forEach((roomMode) => {

const participant2 = await page.context().newPage();
await joinRoom({ page: participant2, roomUrl });
if (browserName === "webkit") {
await new Promise<void>((resolve) => {
page.on("console", (msg: ConsoleMessage) => {
if (
msg.type() === "warning" &&
msg.text() ===
"PeerConnection::addTransceiver with encodings failed, retrying without encodings"
) {
resolve();
}
});
});
}

await expect(participant1.getByTestId("remoteParticipantVideo")).toHaveCount(1);
await expect(participant2.getByTestId("remoteParticipantVideo")).toHaveCount(1);
Expand Down
2 changes: 1 addition & 1 deletion packages/media/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
"dist/**/*.d.cts"
],
"dependencies": {
"@types/ua-parser-js": "^0.7.39",
"check-ip": "^1.1.1",
"events": "^3.3.0",
"ip-address": "^9.0.5",
Expand All @@ -67,6 +66,7 @@
"devDependencies": {
"@babel/core": "^7.23.2",
"@babel/preset-env": "^7.23.2",
"@types/ua-parser-js": "^0.7.39",
"jest-websocket-mock": "^2.5.0",
"process": "^0.11.10"
},
Expand Down
29 changes: 21 additions & 8 deletions packages/media/src/utils/Safari17Handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export class Safari17 extends HandlerInterface {
rtcpMuxPolicy: "require",
...additionalSettings,
},
proprietaryConstraints
proprietaryConstraints,
);

this._pc.addEventListener("icegatheringstatechange", () => {
Expand Down Expand Up @@ -329,18 +329,31 @@ export class Safari17 extends HandlerInterface {
}

const sendingRemoteRtpParameters = utils.clone<RtpParameters>(
this._sendingRemoteRtpParametersByKind![track.kind]
this._sendingRemoteRtpParametersByKind![track.kind],
);

// This may throw.
sendingRemoteRtpParameters.codecs = ortc.reduceCodecs(sendingRemoteRtpParameters.codecs, codec);

const mediaSectionIdx = this._remoteSdp!.getNextMediaSectionIdx();
const transceiver = this._pc.addTransceiver(track, {
direction: "sendonly",
streams: [this._sendStream],
sendEncodings: encodings,
});
let transceiver: RTCRtpTransceiver;
try {
transceiver = this._pc.addTransceiver(track, {
direction: "sendonly",
streams: [this._sendStream],
sendEncodings: encodings,
});
} catch (e) {
if ((e as Error).message === "Type error") {
console.warn("addTransceiver with encodings failed, retrying without encodings");
transceiver = this._pc.addTransceiver(track, {
direction: "sendonly",
streams: [this._sendStream],
});
} else {
throw e;
}
}

if (onRtpSender) {
onRtpSender(transceiver.sender);
Expand All @@ -363,7 +376,7 @@ export class Safari17 extends HandlerInterface {
await this._pc.setLocalDescription(offer);

// We can now get the transceiver.mid.
const localId = transceiver.mid;
const localId = transceiver.mid!;

// Set MID.
sendingRtpParameters.mid = localId;
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -17282,6 +17282,11 @@ ua-parser-js@^1.0.35:
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-1.0.38.tgz#66bb0c4c0e322fe48edfe6d446df6042e62f25e2"
integrity sha512-Aq5ppTOfvrCMgAPneW1HfWj66Xi7XL+/mIy996R1/CLS/rcyJQm6QZdsKrUeivDFQ+Oc9Wyuwor8Ze8peEoUoQ==

ua-parser-js@^1.0.38:
version "1.0.39"
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-1.0.39.tgz#bfc07f361549bf249bd8f4589a4cccec18fd2018"
integrity sha512-k24RCVWlEcjkdOxYmVJgeD/0a1TiSpqLg+ZalVGV9lsnr4yqu0w7tX/x2xX6G4zpkgQnRf89lxuZ1wsbjXM8lw==

ua-parser-js@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-2.0.0.tgz#fae88e352510198bd29a6dd41624c7cd0d2c7ade"
Expand Down

0 comments on commit 8093032

Please sign in to comment.