From 66b74560012ec0bbdcca4dd7f34da7df7b6a2fc1 Mon Sep 17 00:00:00 2001
From: automated-signal <37887102+automated-signal@users.noreply.github.com>
Date: Wed, 21 Feb 2024 22:58:36 -0600
Subject: [PATCH] Fix error handling in makeVideoScreenshot

Co-authored-by: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>
---
 ts/types/VisualAttachment.ts | 70 ++++++++++++++++++------------------
 ts/util/explodePromise.ts    |  4 +--
 2 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/ts/types/VisualAttachment.ts b/ts/types/VisualAttachment.ts
index 59aebbfa3a..67e130393c 100644
--- a/ts/types/VisualAttachment.ts
+++ b/ts/types/VisualAttachment.ts
@@ -10,6 +10,7 @@ import type { LoggerType } from './Logging';
 import { strictAssert } from '../util/assert';
 import { canvasToBlob } from '../util/canvasToBlob';
 import { KIBIBYTE } from './AttachmentSize';
+import { explodePromise } from '../util/explodePromise';
 
 export { blobToArrayBuffer };
 
@@ -208,47 +209,44 @@ export type MakeVideoScreenshotOptionsType = Readonly<{
   logger: Pick<LoggerType, 'error'>;
 }>;
 
-export function makeVideoScreenshot({
+async function loadVideo({
+  objectUrl,
+  logger,
+}: MakeVideoScreenshotOptionsType): Promise<HTMLVideoElement> {
+  const video = document.createElement('video');
+  const { promise, resolve, reject } = explodePromise();
+  video.addEventListener('loadeddata', resolve);
+  video.addEventListener('error', reject);
+  video.src = objectUrl;
+  try {
+    await promise;
+  } catch (error) {
+    logger.error('loadVideo error', toLogFormat(video.error));
+    throw error;
+  } finally {
+    video.removeEventListener('loadeddata', resolve);
+    video.removeEventListener('error', reject);
+  }
+  return video;
+}
+
+export async function makeVideoScreenshot({
   objectUrl,
   contentType = IMAGE_PNG,
   logger,
 }: MakeVideoScreenshotOptionsType): Promise<Blob> {
-  return new Promise((resolve, reject) => {
-    const video = document.createElement('video');
-
-    function seek() {
-      video.currentTime = 1.0;
-    }
-
-    async function capture() {
-      const canvas = document.createElement('canvas');
-      canvas.width = video.videoWidth;
-      canvas.height = video.videoHeight;
-      const context = canvas.getContext('2d');
-      strictAssert(context, 'Failed to get canvas context');
-      context.drawImage(video, 0, 0, canvas.width, canvas.height);
-
-      video.removeEventListener('loadeddata', seek);
-      video.removeEventListener('seeked', capture);
-
-      try {
-        const image = canvasToBlob(canvas, contentType);
-        resolve(image);
-      } catch (err) {
-        reject(err);
-      }
-    }
-
-    video.addEventListener('loadeddata', seek);
-    video.addEventListener('seeked', capture);
-
-    video.addEventListener('error', error => {
-      logger.error('makeVideoScreenshot error', toLogFormat(error));
-      reject(error);
-    });
-
-    video.src = objectUrl;
+  const video = await loadVideo({ objectUrl, logger });
+  await new Promise<unknown>(res => {
+    video.currentTime = 1.0;
+    video.addEventListener('seeked', res, { once: true });
   });
+  const canvas = document.createElement('canvas');
+  canvas.width = video.videoWidth;
+  canvas.height = video.videoHeight;
+  const context = canvas.getContext('2d');
+  strictAssert(context, 'Failed to get canvas context');
+  context.drawImage(video, 0, 0, canvas.width, canvas.height);
+  return canvasToBlob(canvas, contentType);
 }
 
 export function makeObjectUrl(
diff --git a/ts/util/explodePromise.ts b/ts/util/explodePromise.ts
index 2279019d98..447a3b9b35 100644
--- a/ts/util/explodePromise.ts
+++ b/ts/util/explodePromise.ts
@@ -4,12 +4,12 @@
 export type ExplodePromiseResultType<T> = Readonly<{
   promise: Promise<T>;
   resolve: (value: T) => void;
-  reject: (error: Error) => void;
+  reject: (error: unknown) => void;
 }>;
 
 export function explodePromise<T>(): ExplodePromiseResultType<T> {
   let resolve: (value: T) => void;
-  let reject: (error: Error) => void;
+  let reject: (error: unknown) => void;
 
   const promise = new Promise<T>((innerResolve, innerReject) => {
     resolve = innerResolve;