Skip to content

Commit c14d0e7

Browse files
committed
fix: honor the roomOptions passed to Room.connect
`connect` opened with `var roomOptions = this.roomOptions;` while a parameter of the same name was in scope. Dart permits a local to shadow a parameter — silently, with no warning and the local winning — so every `RoomOptions` a caller passed to `connect` was discarded and the Room's own options were used instead. Nothing surfaced the mismatch. The parameter is deprecated in favour of the `Room` constructor, but deprecated is not the same as inert: while it is still accepted it has to take effect. It now does, falling back to the Room's options when absent. `Engine.connect` already adopted whatever it was handed, so the value propagates from there without further changes. The local is renamed to `effectiveRoomOptions`, since restoring the parameter's visibility is the whole point and leaving two things called `roomOptions` in one scope is what caused this. Adds a regression test, and threads `connectOptions`/`roomOptions` through the E2E container so it can be exercised. Verified the test fails against the old shadowing behavior.
1 parent 0a11981 commit c14d0e7

4 files changed

Lines changed: 97 additions & 15 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
patch type="fixed" "Room.connect no longer ignores the roomOptions argument passed to it"

lib/src/core/room.dart

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,12 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
275275
@Deprecated('deprecated, please use roomOptions in Room constructor') RoomOptions? roomOptions,
276276
FastConnectOptions? fastConnectOptions,
277277
}) async {
278-
var roomOptions = this.roomOptions;
279-
if (lkPlatformIs(PlatformType.web) && (roomOptions.networkOptions.certificatePinning?.isEnabled ?? false)) {
278+
// The deprecated `roomOptions` parameter still has to take effect when supplied. It was
279+
// previously shadowed by a local of the same name declared right here — which Dart allows
280+
// silently, with the local winning — so anything callers passed was discarded.
281+
var effectiveRoomOptions = roomOptions ?? this.roomOptions;
282+
if (lkPlatformIs(PlatformType.web) &&
283+
(effectiveRoomOptions.networkOptions.certificatePinning?.isEnabled ?? false)) {
280284
throw UnsupportedError(
281285
'Certificate pinning is not supported on Flutter web, '
282286
'remove certificatePinning from NetworkOptions when targeting web',
@@ -285,13 +289,17 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
285289
connectOptions ??= ConnectOptions();
286290
_pendingTrackQueue.updateTtl(connectOptions.timeouts.subscribe);
287291
// ignore: deprecated_member_use_from_same_package
288-
if ((roomOptions.encryption != null || roomOptions.e2eeOptions != null) && engine.e2eeManager == null) {
292+
if ((effectiveRoomOptions.encryption != null || effectiveRoomOptions.e2eeOptions != null) &&
293+
engine.e2eeManager == null) {
289294
if (!lkPlatformSupportsE2EE()) {
290295
throw LiveKitE2EEException('E2EE is not supported on this platform');
291296
}
292297
// ignore: deprecated_member_use_from_same_package
293-
final e2eeOptions = roomOptions.encryption ?? roomOptions.e2eeOptions;
294-
_e2eeManager = E2EEManager(e2eeOptions!.keyProvider, dcEncryptionEnabled: roomOptions.encryption != null);
298+
final e2eeOptions = effectiveRoomOptions.encryption ?? effectiveRoomOptions.e2eeOptions;
299+
_e2eeManager = E2EEManager(
300+
e2eeOptions!.keyProvider,
301+
dcEncryptionEnabled: effectiveRoomOptions.encryption != null,
302+
);
295303
await _e2eeManager!.setup(this);
296304
engine.setE2eeManager(_e2eeManager);
297305
} else {
@@ -300,8 +308,8 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
300308

301309
if (_e2eeManager != null) {
302310
// Disable backup codec when e2ee is enabled
303-
roomOptions = roomOptions.copyWith(
304-
defaultVideoPublishOptions: roomOptions.defaultVideoPublishOptions.copyWith(
311+
effectiveRoomOptions = effectiveRoomOptions.copyWith(
312+
defaultVideoPublishOptions: effectiveRoomOptions.defaultVideoPublishOptions.copyWith(
305313
backupVideoCodec: const BackupVideoCodec(enabled: false),
306314
),
307315
);
@@ -313,7 +321,11 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
313321
}
314322
if (isCloudUrl(Uri.parse(url))) {
315323
if (_regionUrlProvider == null) {
316-
_regionUrlProvider = RegionUrlProvider(url: url, token: token, networkOptions: roomOptions.networkOptions);
324+
_regionUrlProvider = RegionUrlProvider(
325+
url: url,
326+
token: token,
327+
networkOptions: effectiveRoomOptions.networkOptions,
328+
);
317329
} else {
318330
_regionUrlProvider?.updateToken(token);
319331
}
@@ -336,7 +348,7 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
336348
// AudioManager once, on the first connect. Skipping it on a later manual
337349
// connect of the same Room keeps a runtime speaker change from being
338350
// reverted. New code should call setSpeakerOutputPreferred directly.
339-
final legacySpeakerOn = roomOptions.defaultAudioOutputOptions.speakerOn;
351+
final legacySpeakerOn = effectiveRoomOptions.defaultAudioOutputOptions.speakerOn;
340352
if (legacySpeakerOn != null && !_legacySpeakerBridged && lkPlatformIsMobile()) {
341353
_legacySpeakerBridged = true;
342354
await AudioManager.instance.setSpeakerOutputPreferred(legacySpeakerOn);
@@ -351,7 +363,7 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
351363
_regionUrl ?? url,
352364
token,
353365
connectOptions: connectOptions,
354-
roomOptions: roomOptions,
366+
roomOptions: effectiveRoomOptions,
355367
fastConnectOptions: fastConnectOptions,
356368
regionUrlProvider: _regionUrlProvider,
357369
);
@@ -374,7 +386,7 @@ class Room extends DisposableChangeNotifier with EventsEmittable<RoomEvent> {
374386
nextUrl,
375387
token,
376388
connectOptions: connectOptions,
377-
roomOptions: roomOptions,
389+
roomOptions: effectiveRoomOptions,
378390
fastConnectOptions: fastConnectOptions,
379391
regionUrlProvider: _regionUrlProvider,
380392
);
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright 2026 LiveKit, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
@Timeout(Duration(seconds: 10))
16+
library;
17+
18+
import 'package:flutter_test/flutter_test.dart';
19+
20+
import 'package:livekit_client/livekit_client.dart';
21+
import '../mock/e2e_container.dart';
22+
import '../mock/peerconnection_mock.dart';
23+
24+
void main() {
25+
setUp(resetMockDataChannels);
26+
27+
group('Room.connect options', () {
28+
// Regression: the deprecated `roomOptions` parameter was shadowed by a local of the same name
29+
// in the first line of `connect`, which Dart permits silently. Everything passed here was
30+
// discarded, so callers saw the Room's own options with no indication anything was wrong.
31+
test('honors the roomOptions passed to connect', () async {
32+
final container = E2EContainer(
33+
roomOptions: const RoomOptions(dynacast: false, adaptiveStream: false),
34+
);
35+
addTearDown(container.dispose);
36+
37+
await container.connectRoom(
38+
// ignore: deprecated_member_use_from_same_package
39+
roomOptions: const RoomOptions(dynacast: true, adaptiveStream: true),
40+
);
41+
42+
expect(container.room.roomOptions.dynacast, isTrue);
43+
expect(container.room.roomOptions.adaptiveStream, isTrue);
44+
});
45+
46+
test('falls back to the Room\'s options when connect is given none', () async {
47+
final container = E2EContainer(
48+
roomOptions: const RoomOptions(dynacast: true, adaptiveStream: true),
49+
);
50+
addTearDown(container.dispose);
51+
52+
await container.connectRoom();
53+
54+
expect(container.room.roomOptions.dynacast, isTrue);
55+
expect(container.room.roomOptions.adaptiveStream, isTrue);
56+
});
57+
});
58+
}

test/mock/e2e_container.dart

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ class E2EContainer {
3737
/// since [connectRoom] returned. Populated only when [captureOutbound] is true.
3838
final List<lk_models.DataPacket> capturedDataPackets = [];
3939

40-
E2EContainer() {
40+
E2EContainer({RoomOptions roomOptions = const RoomOptions()}) {
4141
wsConnector = MockWebSocketConnector();
4242
client = SignalClient(wsConnector.connect);
4343
engine = Engine(
4444
connectOptions: const ConnectOptions(),
45-
roomOptions: const RoomOptions(),
45+
roomOptions: roomOptions,
4646
signalClient: client,
4747
peerConnectionCreate: MockPeerConnection.create,
4848
);
@@ -58,8 +58,19 @@ class E2EContainer {
5858
/// that value (used to exercise v1 vs v2 caller paths in self-loop tests).
5959
/// When [captureOutbound] is true, all DataPackets sent over the reliable
6060
/// data channel are recorded in [capturedDataPackets].
61-
Future<void> connectRoom({int? localClientProtocol, bool captureOutbound = false}) async {
62-
final connectFuture = room.connect(exampleUri, token);
61+
Future<void> connectRoom({
62+
int? localClientProtocol,
63+
bool captureOutbound = false,
64+
ConnectOptions? connectOptions,
65+
@Deprecated('mirrors the deprecated Room.connect parameter') RoomOptions? roomOptions,
66+
}) async {
67+
final connectFuture = room.connect(
68+
exampleUri,
69+
token,
70+
connectOptions: connectOptions,
71+
// ignore: deprecated_member_use_from_same_package
72+
roomOptions: roomOptions,
73+
);
6374
Future.delayed(const Duration(milliseconds: 1), () {
6475
final resp = _buildJoinResponse(localClientProtocol);
6576
wsConnector.onData(resp.writeToBuffer());

0 commit comments

Comments
 (0)