mirror of
https://github.com/vector-im/element-call.git
synced 2026-08-02 19:49:23 +00:00
add test reproducing the race
This commit is contained in:
@@ -14,13 +14,19 @@ import {
|
|||||||
type Mock,
|
type Mock,
|
||||||
vi,
|
vi,
|
||||||
} from "vitest";
|
} from "vitest";
|
||||||
import { ConnectionState as LivekitConenctionState } from "livekit-client";
|
import {
|
||||||
import { type BehaviorSubject } from "rxjs";
|
ConnectionState as LivekitConenctionState,
|
||||||
|
LocalParticipant,
|
||||||
|
type LocalTrack,
|
||||||
|
type LocalTrackPublication,
|
||||||
|
} from "livekit-client";
|
||||||
|
import { BehaviorSubject } from "rxjs";
|
||||||
import { logger } from "matrix-js-sdk/lib/logger";
|
import { logger } from "matrix-js-sdk/lib/logger";
|
||||||
|
|
||||||
import { ObservableScope } from "../../ObservableScope";
|
import { ObservableScope } from "../../ObservableScope";
|
||||||
import { constant } from "../../Behavior";
|
import { constant } from "../../Behavior";
|
||||||
import {
|
import {
|
||||||
|
flushPromises,
|
||||||
mockLivekitRoom,
|
mockLivekitRoom,
|
||||||
mockLocalParticipant,
|
mockLocalParticipant,
|
||||||
mockMediaDevices,
|
mockMediaDevices,
|
||||||
@@ -33,8 +39,15 @@ import {
|
|||||||
import { type MuteStates } from "../../MuteStates";
|
import { type MuteStates } from "../../MuteStates";
|
||||||
import { FailToStartLivekitConnection } from "../../../utils/errors";
|
import { FailToStartLivekitConnection } from "../../../utils/errors";
|
||||||
|
|
||||||
|
let scope: ObservableScope;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
scope = new ObservableScope();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => scope.end());
|
||||||
|
|
||||||
describe("Publisher", () => {
|
describe("Publisher", () => {
|
||||||
let scope: ObservableScope;
|
|
||||||
let connection: Connection;
|
let connection: Connection;
|
||||||
let muteStates: MuteStates;
|
let muteStates: MuteStates;
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -50,7 +63,6 @@ describe("Publisher", () => {
|
|||||||
setHandler: vi.fn(),
|
setHandler: vi.fn(),
|
||||||
},
|
},
|
||||||
} as unknown as MuteStates;
|
} as unknown as MuteStates;
|
||||||
scope = new ObservableScope();
|
|
||||||
connection = {
|
connection = {
|
||||||
state$: constant({
|
state$: constant({
|
||||||
state: "ConnectedToLkRoom",
|
state: "ConnectedToLkRoom",
|
||||||
@@ -62,8 +74,6 @@ describe("Publisher", () => {
|
|||||||
} as unknown as Connection;
|
} as unknown as Connection;
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => scope.end());
|
|
||||||
|
|
||||||
it("throws if livekit room could not publish", async () => {
|
it("throws if livekit room could not publish", async () => {
|
||||||
const publisher = new Publisher(
|
const publisher = new Publisher(
|
||||||
scope,
|
scope,
|
||||||
@@ -138,3 +148,110 @@ describe("Publisher", () => {
|
|||||||
).toHaveBeenCalledTimes(3);
|
).toHaveBeenCalledTimes(3);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
|
describe("Bug fix", () => {
|
||||||
|
|
||||||
|
// There is a race condition when creating and publishing tracks while the mute state changes.
|
||||||
|
// This race condition could cause tracks to be published even though they are muted at the
|
||||||
|
// beginning of a call coming from lobby.
|
||||||
|
// This is caused by our stack using manually the low level API to create and publish tracks,
|
||||||
|
// but also using the higher level setMicrophoneEnabled and setCameraEnabled functions that also create
|
||||||
|
// and publish tracks, and managing pending publications.
|
||||||
|
// Race is as follow, on creation of the Publisher we create the tracks then publish them.
|
||||||
|
// If in the middle of that process the mute state changes:
|
||||||
|
// - the `setMicrophoneEnabled` will be no-op because it is not aware of our created track and can't see any pending publication
|
||||||
|
// - If start publication is requested it will publish the track even though there was a mute request.
|
||||||
|
it.fails("wrongly publish tracks while muted", async () => {
|
||||||
|
const audioEnabled$ = new BehaviorSubject(true);
|
||||||
|
const muteStates = {
|
||||||
|
audio: {
|
||||||
|
enabled$: audioEnabled$,
|
||||||
|
unsetHandler: vi.fn(),
|
||||||
|
setHandler: vi.fn(),
|
||||||
|
},
|
||||||
|
video: {
|
||||||
|
enabled$: constant(false),
|
||||||
|
unsetHandler: vi.fn(),
|
||||||
|
setHandler: vi.fn(),
|
||||||
|
},
|
||||||
|
} as unknown as MuteStates;
|
||||||
|
|
||||||
|
const mockSendDataPacket = vi.fn();
|
||||||
|
const mockEngine = {
|
||||||
|
client: {
|
||||||
|
sendUpdateLocalMetadata: vi.fn(),
|
||||||
|
},
|
||||||
|
on: vi.fn().mockReturnThis(),
|
||||||
|
sendDataPacket: mockSendDataPacket,
|
||||||
|
};
|
||||||
|
|
||||||
|
// cont mockRoomOptions = {} as InternalRoomOptions;
|
||||||
|
|
||||||
|
const localParticipant = new LocalParticipant(
|
||||||
|
"local-sid",
|
||||||
|
"local-identity",
|
||||||
|
// @ts-expect-error - for that test we want a real LocalParticipant to have the pending publications logic
|
||||||
|
mockEngine,
|
||||||
|
{},
|
||||||
|
new Map(),
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
|
||||||
|
const connection = {
|
||||||
|
state$: constant({
|
||||||
|
state: "ConnectedToLkRoom",
|
||||||
|
livekitConnectionState$: constant(LivekitConenctionState.Connected),
|
||||||
|
}),
|
||||||
|
livekitRoom: mockLivekitRoom({
|
||||||
|
localParticipant,
|
||||||
|
}),
|
||||||
|
} as unknown as Connection;
|
||||||
|
|
||||||
|
const mediaDevices = mockMediaDevices({});
|
||||||
|
|
||||||
|
const mockTrack = vi.mocked<LocalTrack>({
|
||||||
|
kind: "audio",
|
||||||
|
mute: vi.fn(),
|
||||||
|
} as Partial<LocalTrack> as LocalTrack);
|
||||||
|
const createTrackLock = Promise.withResolvers<void>();
|
||||||
|
const createTrackSpy = vi.spyOn(localParticipant, "createTracks");
|
||||||
|
createTrackSpy.mockImplementation(async () => {
|
||||||
|
await createTrackLock.promise;
|
||||||
|
return [mockTrack];
|
||||||
|
});
|
||||||
|
|
||||||
|
const publishTrackSpy = vi.spyOn(localParticipant, "publishTrack");
|
||||||
|
publishTrackSpy.mockResolvedValue({} as unknown as LocalTrackPublication);
|
||||||
|
|
||||||
|
const publisher = new Publisher(
|
||||||
|
scope,
|
||||||
|
connection,
|
||||||
|
mediaDevices,
|
||||||
|
muteStates,
|
||||||
|
constant({ supported: false, processor: undefined }),
|
||||||
|
logger,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Initially the audio is unmuted, so creating tracks should publish the audio track
|
||||||
|
const createTracks = publisher.createAndSetupTracks();
|
||||||
|
publisher.tracks$.subscribe(() => {
|
||||||
|
void publisher.startPublishing();
|
||||||
|
});
|
||||||
|
// now mute the audio before allowing track creation to complete
|
||||||
|
audioEnabled$.next(false);
|
||||||
|
// const publishing = publisher.startPublishing();
|
||||||
|
createTrackLock.resolve();
|
||||||
|
await createTracks;
|
||||||
|
// await publishing;
|
||||||
|
|
||||||
|
await flushPromises();
|
||||||
|
|
||||||
|
// It should not publish or instead call track.mute()
|
||||||
|
try {
|
||||||
|
expect(publishTrackSpy).not.toHaveBeenCalled();
|
||||||
|
} catch {
|
||||||
|
expect(mockTrack.mute).toHaveBeenCalled();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user