From 533b250bb6f0abcc2ca82d839120ff850bcc538f Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 24 Jan 2023 11:20:26 +0100 Subject: [PATCH] Handle broadcast chunk errors (#9970) * Use strings for broadcast playback states * Handle broadcast decode errors --- src/i18n/strings/en_EN.json | 1 + .../components/atoms/VoiceBroadcastError.tsx | 32 ++ .../molecules/VoiceBroadcastPlaybackBody.tsx | 30 +- src/voice-broadcast/index.ts | 1 + .../models/VoiceBroadcastPlayback.ts | 80 ++- ...oiceBroadcastPlaybackControl-test.tsx.snap | 36 +- .../VoiceBroadcastPlaybackBody-test.tsx | 18 +- .../VoiceBroadcastPlaybackBody-test.tsx.snap | 521 ++++++++++-------- ...ceBroadcastSmallPlaybackBody-test.tsx.snap | 4 +- .../models/VoiceBroadcastPlayback-test.tsx | 35 +- 10 files changed, 483 insertions(+), 275 deletions(-) create mode 100644 src/voice-broadcast/components/atoms/VoiceBroadcastError.tsx diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index c932fab0c3..a8e1a3a7ee 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -659,6 +659,7 @@ "%(senderName)s ended a voice broadcast": "%(senderName)s ended a voice broadcast", "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", diff --git a/src/voice-broadcast/components/atoms/VoiceBroadcastError.tsx b/src/voice-broadcast/components/atoms/VoiceBroadcastError.tsx new file mode 100644 index 0000000000..8e5a4481cb --- /dev/null +++ b/src/voice-broadcast/components/atoms/VoiceBroadcastError.tsx @@ -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 = ({ message }) => { + return ( +
+ + {message} +
+ ); +}; diff --git a/src/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody.tsx b/src/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody.tsx index d25c6f3539..a3b94824a9 100644 --- a/src/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody.tsx +++ b/src/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody.tsx @@ -18,6 +18,7 @@ import React, { ReactElement } from "react"; import classNames from "classnames"; import { + VoiceBroadcastError, VoiceBroadcastHeader, VoiceBroadcastPlayback, VoiceBroadcastPlaybackControl, @@ -67,6 +68,24 @@ export const VoiceBroadcastPlaybackBody: React.FC + ) : ( + <> +
+ {seekBackwardButton} + + {seekForwardButton} +
+ +
+ + +
+ + ); + return (
-
- {seekBackwardButton} - - {seekForwardButton} -
- -
- - -
+ {content}
); }; diff --git a/src/voice-broadcast/index.ts b/src/voice-broadcast/index.ts index cd7a6a091b..57b29dc8f4 100644 --- a/src/voice-broadcast/index.ts +++ b/src/voice-broadcast/index.ts @@ -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"; diff --git a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts index 9d04dd7394..80ec38bd75 100644 --- a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts +++ b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts @@ -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 { @@ -205,12 +207,24 @@ export class VoiceBroadcastPlayback } }; + private async tryLoadPlayback(chunkEvent: MatrixEvent): Promise { + 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 { 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); @@ -311,16 +325,28 @@ export class VoiceBroadcastPlayback private async playEvent(event: MatrixEvent): Promise { 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 { + 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 { 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)) { @@ -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; } @@ -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); @@ -464,6 +489,9 @@ 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; @@ -471,6 +499,9 @@ export class VoiceBroadcastPlayback } 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; @@ -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(); @@ -496,6 +530,9 @@ export class VoiceBroadcastPlayback * paused → playing */ public async toggle(): Promise { + // error is a final state + if (this.getState() === VoiceBroadcastPlaybackState.Error) return; + if (this.state === VoiceBroadcastPlaybackState.Stopped) { await this.start(); return; @@ -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; } @@ -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; } @@ -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(); diff --git a/test/voice-broadcast/components/atoms/__snapshots__/VoiceBroadcastPlaybackControl-test.tsx.snap b/test/voice-broadcast/components/atoms/__snapshots__/VoiceBroadcastPlaybackControl-test.tsx.snap index f02e919295..cf954d38df 100644 --- a/test/voice-broadcast/components/atoms/__snapshots__/VoiceBroadcastPlaybackControl-test.tsx.snap +++ b/test/voice-broadcast/components/atoms/__snapshots__/VoiceBroadcastPlaybackControl-test.tsx.snap @@ -1,6 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` should render state 0 as expected 1`] = ` +exports[` should render state buffering as expected 1`] = ` +
+
+
+
+
+`; + +exports[` should render state pause as expected 1`] = `
should render state 0 as expected 1`]
`; -exports[` should render state 1 as expected 1`] = ` +exports[` should render state playing as expected 1`] = `
should render state 1 as expected 1`]
`; -exports[` should render state 2 as expected 1`] = ` +exports[` should render state stopped as expected 1`] = `
should render state 2 as expected 1`]
`; - -exports[` should render state 3 as expected 1`] = ` -
-
-
-
-
-`; diff --git a/test/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody-test.tsx b/test/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody-test.tsx index 540653b1c2..15cf39f443 100644 --- a/test/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody-test.tsx +++ b/test/voice-broadcast/components/molecules/VoiceBroadcastPlaybackBody-test.tsx @@ -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"; @@ -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(); @@ -214,6 +219,17 @@ describe("VoiceBroadcastPlaybackBody", () => { }); }); + describe("when rendering an error broadcast", () => { + beforeEach(() => { + mocked(playback.getState).mockReturnValue(VoiceBroadcastPlaybackState.Error); + renderResult = render(); + }); + + it("should render as expected", () => { + expect(renderResult.container).toMatchSnapshot(); + }); + }); + describe.each([ [VoiceBroadcastPlaybackState.Paused, "not-live"], [VoiceBroadcastPlaybackState.Playing, "live"], diff --git a/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastPlaybackBody-test.tsx.snap b/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastPlaybackBody-test.tsx.snap index 2ead27d5a2..7539d4bef8 100644 --- a/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastPlaybackBody-test.tsx.snap +++ b/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastPlaybackBody-test.tsx.snap @@ -1,235 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`VoiceBroadcastPlaybackBody when rendering a 0/not-live broadcast should render as expected 1`] = ` -
-
-
-
- room avatar: - My room -
-
-
-
- My room -
-
-
-
- - @user:example.com - -
-
-
- Voice broadcast -
-
-
-
-
-
-
-
-
-
-
-
-
-
- -
- - 00:00 - - - -23:42 - -
-
-
-`; - -exports[`VoiceBroadcastPlaybackBody when rendering a 1/live broadcast should render as expected 1`] = ` -
-
-
-
- room avatar: - My room -
-
-
-
- My room -
-
-
-
- - @user:example.com - -
-
-
- Voice broadcast -
-
-
-
- Live -
-
-
-
-
-
-
-
-
-
-
-
-
- -
- - 00:00 - - - -23:42 - -
-
-
-`; - exports[`VoiceBroadcastPlaybackBody when rendering a buffering voice broadcast should render as expected 1`] = `
`; +exports[`VoiceBroadcastPlaybackBody when rendering a pause/not-live broadcast should render as expected 1`] = ` +
+
+
+
+ room avatar: + My room +
+
+
+
+ My room +
+
+
+
+ + @user:example.com + +
+
+
+ Voice broadcast +
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+ + 00:00 + + + -23:42 + +
+
+
+`; + exports[`VoiceBroadcastPlaybackBody when rendering a playing broadcast in pip mode should render as expected 1`] = `
`; +exports[`VoiceBroadcastPlaybackBody when rendering a playing/live broadcast should render as expected 1`] = ` +
+
+
+
+ room avatar: + My room +
+
+
+
+ My room +
+
+
+
+ + @user:example.com + +
+
+
+ Voice broadcast +
+
+
+
+ Live +
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+ + 00:00 + + + -23:42 + +
+
+
+`; + exports[`VoiceBroadcastPlaybackBody when rendering a stopped broadcast should render as expected 1`] = `
`; + +exports[`VoiceBroadcastPlaybackBody when rendering an error broadcast should render as expected 1`] = ` +
+
+
+
+ room avatar: + My room +
+
+
+
+ My room +
+
+
+
+ + @user:example.com + +
+
+
+ Voice broadcast +
+
+
+
+
+ Unable to play this voice broadcast +
+
+
+`; diff --git a/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastSmallPlaybackBody-test.tsx.snap b/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastSmallPlaybackBody-test.tsx.snap index 27ad4098a2..f0319c6e73 100644 --- a/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastSmallPlaybackBody-test.tsx.snap +++ b/test/voice-broadcast/components/molecules/__snapshots__/VoiceBroadcastSmallPlaybackBody-test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` when rendering a { state: 0, liveness: 'not-live' }/%s broadcast should render as expected 1`] = ` +exports[` when rendering a { state: 'pause', liveness: 'not-live' }/%s broadcast should render as expected 1`] = `
when rendering a { state: 0, livene
`; -exports[` when rendering a { state: 1, liveness: 'live' }/%s broadcast should render as expected 1`] = ` +exports[` when rendering a { state: 'playing', liveness: 'live' }/%s broadcast should render as expected 1`] = `
{ }); }; - filterConsole("Starting load of AsyncWrapper for modal"); + filterConsole( + "Starting load of AsyncWrapper for modal", + // expected for some tests + "Unable to load broadcast playback", + ); beforeEach(() => { client = stubClient(); @@ -283,6 +287,35 @@ describe("VoiceBroadcastPlayback", () => { expect(playback.durationSeconds).toEqual(6.5); }); + describe("and starting a playback with a broken chunk", () => { + beforeEach(async () => { + mocked(chunk2Playback.prepare).mockRejectedValue("Error decoding chunk"); + await playback.start(); + }); + + itShouldSetTheStateTo(VoiceBroadcastPlaybackState.Error); + + it("start() should keep it in the error state)", async () => { + await playback.start(); + expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Error); + }); + + it("stop() should keep it in the error state)", () => { + playback.stop(); + expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Error); + }); + + it("toggle() should keep it in the error state)", async () => { + await playback.toggle(); + expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Error); + }); + + it("pause() should keep it in the error state)", () => { + playback.pause(); + expect(playback.getState()).toBe(VoiceBroadcastPlaybackState.Error); + }); + }); + describe("and an event with the same transaction Id occurs", () => { beforeEach(() => { room.addLiveEvents([chunk2BEvent]);