Keep a copy of the audio buffer so the WAV fallback can run (#34481)

* Keep a copy of the audio buffer so the WAV fallback can run

decodeAudioData detaches the buffer it is given, so the catch block handed an
already detached buffer to decodeOgg and it threw "Cannot perform Construct on
a detached ArrayBuffer" before it could re-encode. The fallback could never
run and the user saw an error instead of their voice message.

Fixes https://github.com/element-hq/element-web/issues/24904

* Assert the fallback buffer is usable instead of constructing a view for its side effect

---------

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>

apps/web/src/audio/Playback.ts

@@ -168,6 +168,9 @@ export class Playback extends EventEmitter implements IDestroyable, PlaybackInte
             this.element.src = URL.createObjectURL(new Blob([this.buf]));
             await deferred.promise; // make sure the audio element is ready for us
         } else {
+            // decodeAudioData detaches the buffer it is given, so the copy the fallback needs has
+            // to be taken before we call it rather than inside the error handler.
+            const fallbackBuf = this.buf.slice(0);
             try {
                 this.audioBuf = await this.context.decodeAudioData(this.buf);
             } catch (e) {
@@ -176,7 +179,7 @@ export class Playback extends EventEmitter implements IDestroyable, PlaybackInte
 
                 try {
                     // This error handler is largely for Safari, which doesn't support Opus/Ogg very well.
-                    const wav = await decodeOgg(this.buf);
+                    const wav = await decodeOgg(fallbackBuf);
                     this.audioBuf = await this.context.decodeAudioData(wav);
                 } catch (e) {
                     logger.error("Error decoding recording:", e);

apps/web/test/unit-tests/audio/Playback-test.ts

@@ -176,6 +176,34 @@ describe("Playback", () => {
             expect(playback.currentState).toEqual(PlaybackState.Stopped);
         });
 
+        it("hands the ogg fallback a buffer which decodeAudioData has not detached", async () => {
+            // stub logger to keep console clean from expected error
+            jest.spyOn(logger, "error").mockReturnValue(undefined);
+            jest.spyOn(logger, "warn").mockReturnValue(undefined);
+
+            const buffer = new ArrayBuffer(8);
+            mockAudioContext.decodeAudioData
+                .mockImplementationOnce((buf: ArrayBuffer) => {
+                    // The real decodeAudioData detaches the buffer it is handed, even when it fails.
+                    structuredClone(buf, { transfer: [buf] });
+                    return Promise.reject(new Error("test"));
+                })
+                .mockResolvedValueOnce(mockAudioBuffer);
+            // Constructing a view over a detached buffer throws, which is what decodeOgg does first.
+            mocked(decodeOgg).mockImplementationOnce(async (audioBuffer: ArrayBuffer) => {
+                expect(() => new Uint8Array(audioBuffer)).not.toThrow();
+                return new ArrayBuffer(1);
+            });
+
+            const playback = new Playback(buffer);
+
+            await playback.prepare();
+
+            expect(decodeOgg).toHaveBeenCalled();
+            expect(mockAudioContext.decodeAudioData).toHaveBeenCalledTimes(2);
+            expect(playback.currentState).toEqual(PlaybackState.Stopped);
+        });
+
         it("does not try to re-decode audio", async () => {
             const buffer = new ArrayBuffer(8);
             const playback = new Playback(buffer);