Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Handle broadcast chunk errors (#9970)
Browse files Browse the repository at this point in the history
* Use strings for broadcast playback states

* Handle broadcast decode errors
  • Loading branch information
weeman1337 authored Jan 24, 2023
1 parent 60edb85 commit 533b250
Show file tree
Hide file tree
Showing 10 changed files with 312 additions and 104 deletions.
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@
"%(senderName)s ended a <a>voice broadcast</a>": "%(senderName)s ended a <a>voice broadcast</a>",
"You ended a voice broadcast": "You ended a voice broadcast",
"%(senderName)s ended a voice broadcast": "%(senderName)s ended a voice broadcast",
"Unable to play this voice broadcast": "Unable to play this voice broadcast",
"Stop live broadcasting?": "Stop live broadcasting?",
"Are you sure you want to stop your live broadcast?This will end the broadcast and the full recording will be available in the room.": "Are you sure you want to stop your live broadcast?This will end the broadcast and the full recording will be available in the room.",
"Yes, stop broadcast": "Yes, stop broadcast",
Expand Down
32 changes: 32 additions & 0 deletions src/voice-broadcast/components/atoms/VoiceBroadcastError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";

import { Icon as WarningIcon } from "../../../../res/img/element-icons/warning.svg";

interface Props {
message: string;
}

export const VoiceBroadcastError: React.FC<Props> = ({ message }) => {
return (
<div className="mx_VoiceBroadcastRecordingConnectionError">
<WarningIcon className="mx_Icon mx_Icon_16" />
{message}
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import React, { ReactElement } from "react";
import classNames from "classnames";

import {
VoiceBroadcastError,
VoiceBroadcastHeader,
VoiceBroadcastPlayback,
VoiceBroadcastPlaybackControl,
Expand Down Expand Up @@ -67,6 +68,24 @@ export const VoiceBroadcastPlaybackBody: React.FC<VoiceBroadcastPlaybackBodyProp
["mx_VoiceBroadcastBody--pip"]: pip,
});

const content =
playbackState === VoiceBroadcastPlaybackState.Error ? (
<VoiceBroadcastError message={playback.errorMessage} />
) : (
<>
<div className="mx_VoiceBroadcastBody_controls">
{seekBackwardButton}
<VoiceBroadcastPlaybackControl state={playbackState} onClick={toggle} />
{seekForwardButton}
</div>
<SeekBar playback={playback} />
<div className="mx_VoiceBroadcastBody_timerow">
<Clock seconds={times.position} />
<Clock seconds={-times.timeLeft} />
</div>
</>
);

return (
<div className={classes}>
<VoiceBroadcastHeader
Expand All @@ -77,16 +96,7 @@ export const VoiceBroadcastPlaybackBody: React.FC<VoiceBroadcastPlaybackBodyProp
showBroadcast={playbackState !== VoiceBroadcastPlaybackState.Buffering}
showBuffering={playbackState === VoiceBroadcastPlaybackState.Buffering}
/>
<div className="mx_VoiceBroadcastBody_controls">
{seekBackwardButton}
<VoiceBroadcastPlaybackControl state={playbackState} onClick={toggle} />
{seekForwardButton}
</div>
<SeekBar playback={playback} />
<div className="mx_VoiceBroadcastBody_timerow">
<Clock seconds={times.position} />
<Clock seconds={-times.timeLeft} />
</div>
{content}
</div>
);
};
1 change: 1 addition & 0 deletions src/voice-broadcast/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export * from "./audio/VoiceBroadcastRecorder";
export * from "./components/VoiceBroadcastBody";
export * from "./components/atoms/LiveBadge";
export * from "./components/atoms/VoiceBroadcastControl";
export * from "./components/atoms/VoiceBroadcastError";
export * from "./components/atoms/VoiceBroadcastHeader";
export * from "./components/atoms/VoiceBroadcastPlaybackControl";
export * from "./components/atoms/VoiceBroadcastRecordingConnectionError";
Expand Down
80 changes: 67 additions & 13 deletions src/voice-broadcast/models/VoiceBroadcastPlayback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ import {
import { RelationsHelper, RelationsHelperEvent } from "../../events/RelationsHelper";
import { VoiceBroadcastChunkEvents } from "../utils/VoiceBroadcastChunkEvents";
import { determineVoiceBroadcastLiveness } from "../utils/determineVoiceBroadcastLiveness";
import { _t } from "../../languageHandler";

export enum VoiceBroadcastPlaybackState {
Paused,
Playing,
Stopped,
Buffering,
Paused = "pause",
Playing = "playing",
Stopped = "stopped",
Buffering = "buffering",
Error = "error",
}

export enum VoiceBroadcastPlaybackEvent {
Expand Down Expand Up @@ -205,12 +207,24 @@ export class VoiceBroadcastPlayback
}
};

private async tryLoadPlayback(chunkEvent: MatrixEvent): Promise<void> {
try {
return await this.loadPlayback(chunkEvent);
} catch (err) {
logger.warn("Unable to load broadcast playback", {
message: err.message,
broadcastId: this.infoEvent.getId(),
chunkId: chunkEvent.getId(),
});
this.setError();
}
}

private async loadPlayback(chunkEvent: MatrixEvent): Promise<void> {
const eventId = chunkEvent.getId();

if (!eventId) {
logger.warn("got voice broadcast chunk event without ID", this.infoEvent, chunkEvent);
return;
throw new Error("Broadcast chunk event without Id occurred");
}

const helper = new MediaEventHelper(chunkEvent);
Expand Down Expand Up @@ -311,16 +325,28 @@ export class VoiceBroadcastPlayback
private async playEvent(event: MatrixEvent): Promise<void> {
this.setState(VoiceBroadcastPlaybackState.Playing);
this.currentlyPlaying = event;
const playback = await this.getOrLoadPlaybackForEvent(event);
const playback = await this.tryGetOrLoadPlaybackForEvent(event);
playback?.play();
}

private async tryGetOrLoadPlaybackForEvent(event: MatrixEvent): Promise<Playback | undefined> {
try {
return await this.getOrLoadPlaybackForEvent(event);
} catch (err) {
logger.warn("Unable to load broadcast playback", {
message: err.message,
broadcastId: this.infoEvent.getId(),
chunkId: event.getId(),
});
this.setError();
}
}

private async getOrLoadPlaybackForEvent(event: MatrixEvent): Promise<Playback | undefined> {
const eventId = event.getId();

if (!eventId) {
logger.warn("event without id occurred");
return;
throw new Error("Broadcast chunk event without Id occurred");
}

if (!this.playbacks.has(eventId)) {
Expand All @@ -330,13 +356,12 @@ export class VoiceBroadcastPlayback
const playback = this.playbacks.get(eventId);

if (!playback) {
// logging error, because this should not happen
logger.warn("unable to find playback for event", event);
throw new Error(`Unable to find playback for event ${event.getId()}`);
}

// try to load the playback for the next event for a smooth(er) playback
const nextEvent = this.chunkEvents.getNext(event);
if (nextEvent) this.loadPlayback(nextEvent);
if (nextEvent) this.tryLoadPlayback(nextEvent);

return playback;
}
Expand Down Expand Up @@ -405,8 +430,8 @@ export class VoiceBroadcastPlayback
}

const currentPlayback = this.getCurrentPlayback();
const skipToPlayback = await this.tryGetOrLoadPlaybackForEvent(event);
const currentPlaybackEvent = this.currentlyPlaying;
const skipToPlayback = await this.getOrLoadPlaybackForEvent(event);

if (!skipToPlayback) {
logger.warn("voice broadcast chunk to skip to not found", event);
Expand Down Expand Up @@ -464,13 +489,19 @@ export class VoiceBroadcastPlayback
}

public stop(): void {
// error is a final state
if (this.getState() === VoiceBroadcastPlaybackState.Error) return;

this.setState(VoiceBroadcastPlaybackState.Stopped);
this.getCurrentPlayback()?.stop();
this.currentlyPlaying = null;
this.setPosition(0);
}

public pause(): void {
// error is a final state
if (this.getState() === VoiceBroadcastPlaybackState.Error) return;

// stopped voice broadcasts cannot be paused
if (this.getState() === VoiceBroadcastPlaybackState.Stopped) return;

Expand All @@ -479,6 +510,9 @@ export class VoiceBroadcastPlayback
}

public resume(): void {
// error is a final state
if (this.getState() === VoiceBroadcastPlaybackState.Error) return;

if (!this.currentlyPlaying) {
// no playback to resume, start from the beginning
this.start();
Expand All @@ -496,6 +530,9 @@ export class VoiceBroadcastPlayback
* paused → playing
*/
public async toggle(): Promise<void> {
// error is a final state
if (this.getState() === VoiceBroadcastPlaybackState.Error) return;

if (this.state === VoiceBroadcastPlaybackState.Stopped) {
await this.start();
return;
Expand All @@ -514,6 +551,9 @@ export class VoiceBroadcastPlayback
}

private setState(state: VoiceBroadcastPlaybackState): void {
// error is a final state
if (this.getState() === VoiceBroadcastPlaybackState.Error) return;

if (this.state === state) {
return;
}
Expand All @@ -522,6 +562,16 @@ export class VoiceBroadcastPlayback
this.emit(VoiceBroadcastPlaybackEvent.StateChanged, state, this);
}

/**
* Set error state. Stop current playback, if any.
*/
private setError(): void {
this.setState(VoiceBroadcastPlaybackState.Error);
this.getCurrentPlayback()?.stop();
this.currentlyPlaying = null;
this.setPosition(0);
}

public getInfoState(): VoiceBroadcastInfoState {
return this.infoState;
}
Expand All @@ -536,6 +586,10 @@ export class VoiceBroadcastPlayback
this.setLiveness(determineVoiceBroadcastLiveness(this.infoState));
}

public get errorMessage(): string {
return this.getState() === VoiceBroadcastPlaybackState.Error ? _t("Unable to play this voice broadcast") : "";
}

public destroy(): void {
this.chunkRelationHelper.destroy();
this.infoRelationHelper.destroy();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<VoiceBroadcastPlaybackControl /> should render state 0 as expected 1`] = `
exports[`<VoiceBroadcastPlaybackControl /> should render state buffering as expected 1`] = `
<div>
<div
aria-label="resume voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl mx_VoiceBroadcastControl-play"
aria-label="pause voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl"
role="button"
tabindex="0"
>
Expand All @@ -15,11 +15,11 @@ exports[`<VoiceBroadcastPlaybackControl /> should render state 0 as expected 1`]
</div>
`;

exports[`<VoiceBroadcastPlaybackControl /> should render state 1 as expected 1`] = `
exports[`<VoiceBroadcastPlaybackControl /> should render state pause as expected 1`] = `
<div>
<div
aria-label="pause voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl"
aria-label="resume voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl mx_VoiceBroadcastControl-play"
role="button"
tabindex="0"
>
Expand All @@ -30,11 +30,11 @@ exports[`<VoiceBroadcastPlaybackControl /> should render state 1 as expected 1`]
</div>
`;

exports[`<VoiceBroadcastPlaybackControl /> should render state 2 as expected 1`] = `
exports[`<VoiceBroadcastPlaybackControl /> should render state playing as expected 1`] = `
<div>
<div
aria-label="play voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl mx_VoiceBroadcastControl-play"
aria-label="pause voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl"
role="button"
tabindex="0"
>
Expand All @@ -45,11 +45,11 @@ exports[`<VoiceBroadcastPlaybackControl /> should render state 2 as expected 1`]
</div>
`;

exports[`<VoiceBroadcastPlaybackControl /> should render state 3 as expected 1`] = `
exports[`<VoiceBroadcastPlaybackControl /> should render state stopped as expected 1`] = `
<div>
<div
aria-label="pause voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl"
aria-label="play voice broadcast"
class="mx_AccessibleButton mx_VoiceBroadcastControl mx_VoiceBroadcastControl-play"
role="button"
tabindex="0"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
VoiceBroadcastPlaybackEvent,
VoiceBroadcastPlaybackState,
} from "../../../../src/voice-broadcast";
import { stubClient } from "../../../test-utils";
import { filterConsole, stubClient } from "../../../test-utils";
import { mkVoiceBroadcastInfoStateEvent } from "../../utils/test-utils";
import dis from "../../../../src/dispatcher/dispatcher";
import { Action } from "../../../../src/dispatcher/actions";
Expand All @@ -53,6 +53,11 @@ describe("VoiceBroadcastPlaybackBody", () => {
let playback: VoiceBroadcastPlayback;
let renderResult: RenderResult;

filterConsole(
// expected for some tests
"voice broadcast chunk event to skip to not found",
);

beforeAll(() => {
client = stubClient();
mocked(client.relations).mockClear();
Expand Down Expand Up @@ -214,6 +219,17 @@ describe("VoiceBroadcastPlaybackBody", () => {
});
});

describe("when rendering an error broadcast", () => {
beforeEach(() => {
mocked(playback.getState).mockReturnValue(VoiceBroadcastPlaybackState.Error);
renderResult = render(<VoiceBroadcastPlaybackBody playback={playback} />);
});

it("should render as expected", () => {
expect(renderResult.container).toMatchSnapshot();
});
});

describe.each([
[VoiceBroadcastPlaybackState.Paused, "not-live"],
[VoiceBroadcastPlaybackState.Playing, "live"],
Expand Down
Loading

0 comments on commit 533b250

Please sign in to comment.