Carousels: deliver every video, and stop shouting about photos

e2e.sh

@@ -1,8 +1,8 @@
 #!/bin/sh
 # Usage: ./e2e.sh [full] [-u]
 #   full  also test rate-limit-prone sites (youtube rejects more than a few
-#         hits per hour). Used by the deploy gate (prod.sh); the pre-push
-#         hook runs the reduced set.
+#         hits per hour) and the slow multi-video cases. Used by the deploy
+#         gate (prod.sh); the pre-push hook runs the reduced set.
 #   -u    refresh snapshots. Implies full: `bun test -u` deletes snapshots of
 #         tests it didn't run, so a reduced -u run would silently prune the
 #         full-mode snapshots.

src/blob-store.ts

@@ -12,6 +12,11 @@ const envDir = Bun.env.BLOB_DIR || '/storage/blobs/';
 const BLOB_DIR = envDir.endsWith('/') ? envDir : `${envDir}/`;
 await mkdir(BLOB_DIR, { recursive: true });
 
+// the generic extractor's id is just the URL basename: two hosts' /video.mp4
+// both yield id 'video'
+const hasVideoIdentity = (info: VideoInfo): boolean =>
+  !!(info.extractor && info.id && info.extractor !== 'generic');
+
 // A blob's key is yt-dlp's stable per-video identity, extractor:id:format
 // (known from --dump-json before the download), so a blob we already have is found
 // without re-downloading, and two URLs for one video resolve to the same key.
@@ -21,16 +26,21 @@ await mkdir(BLOB_DIR, { recursive: true });
 // with colliding titles would otherwise share a key (and, worse, the first one's
 // cached file_id). This is the raw DB key; blobName turns it into the on-disk
 // filename.
-// the generic extractor's id is just the URL basename (verified against the
-// real yt-dlp: two hosts' /video.mp4 both yield id 'video'), so it is NOT an
-// identity; those go through the URL-salted fallback like identity-less infos
 export const blobKey = (info: VideoInfo): string =>
-  info.extractor && info.id && info.extractor !== 'generic'
+  hasVideoIdentity(info)
     ? `${info.extractor}:${info.id}:${info.format_id ?? ''}`
     : info.webpage_url
       ? `${info.filename}:${info.webpage_url}`
       : info.filename;
 
+// The fallback cannot reuse blobKey's filename, which carries the format id (see
+// yt-dlp.conf's --output); yt-dlp suffixes " (N)" to the titles of one page's
+// entries, so title still separates them.
+export const videoKey = (info: VideoInfo): string =>
+  hasVideoIdentity(info)
+    ? `${info.extractor}:${info.id}`
+    : `${info.title}:${info.id ?? ''}:${info.webpage_url ?? ''}`;
+
 const extOf = (info: VideoInfo) => {
   const e =
     info.ext ||

src/db.ts

@@ -82,6 +82,9 @@ export const MIGRATIONS: string[] = [
    UPDATE video_info SET webpage_url = json_extract(info, '$.webpage_url');
    CREATE INDEX video_info_webpage_col ON video_info (webpage_url);
    DROP INDEX video_info_webpage;`,
+  // `info` holds the JSON array of a post's video entries; rows predating that
+  // hold one bare object. A six-hour cache, so drop them instead of converting.
+  `DELETE FROM video_info;`,
 ];
 
 // Exported so the migration tests replay THIS loop against a scratch DB (a

src/download-video.ts

@@ -38,45 +38,59 @@ const getErrorMessage = (proc: Bun.ReadableSubprocess) =>
       ? `yt-dlp was killed with signal ${proc.signalCode}`
       : `yt-dlp exited with code ${proc.exitCode}`;
 
+const errorLines = (stderr: string) =>
+  stderr.split('\n').filter((line) => line.startsWith('ERROR:'));
+
 // carries the failing yt-dlp's stderr so callers can classify it (see
 // isPermanentError)
 export class YtdlpError extends Error {
+  declare readonly stdout: string;
   constructor(
     message: string,
     readonly stderr: string,
     // killed by a signal (timeout/OOM) rather than exiting with a code
     readonly signalled = false,
+    stdout = '',
   ) {
-    // yt-dlp's own ERROR: line (the last one is the fatal one) says WHY it
-    // failed; the exit code alone helps nobody, so it's only the fallback.
+    // yt-dlp's own ERROR: line says WHY it failed; the exit code alone helps
+    // nobody, so it's only the fallback.
     // A signal kill keeps its message: the timeout is the real story there,
     // and any ERROR line in a killed process's output is a stale partial.
-    // De-noise for the report: drop the [extractor] tag and the "(caused
-    // by ...)" suffix that repeats the main clause; the raw line still
-    // streams to the chat verbatim, and classification reads raw stderr.
-    const errLine = signalled
-      ? undefined
-      : stderr
-          .split('\n')
-          .findLast((line) => line.startsWith('ERROR:'))
-          ?.slice('ERROR:'.length)
-          .replace(/^\s*\[[^\]]+\]\s*/, '')
-          .replace(/\s*\(caused by .*\)\s*$/, '')
-          .trim();
+    const lines = signalled ? [] : errorLines(stderr);
+    const errLine = (lines.findLast((l) => !PHOTO_ITEM.test(l)) ?? lines.at(-1))
+      ?.slice('ERROR:'.length)
+      // De-noising is lossless: the raw line still streams to the chat
+      // verbatim, and classification reads raw stderr.
+      .replace(/^\s*\[[^\]]+\]\s*/, '')
+      // yt-dlp appends this after any "(caused by ...)", so it has to be
+      // stripped first
+      .replace(
+        /[;,]?\s*(?:if you believe this is an error,\s*)?please report this issue on .*/i,
+        '',
+      )
+      .replace(/\s*\(caused by .*\)\s*$/, '')
+      .trim();
     super(errLine || message);
     this.name = 'YtdlpError';
+    // hidden from console.error, which prints an Error's own properties whole:
+    // a playlist's dump-json runs to megabytes (see STDERR_TAIL for the cap on
+    // the other half)
+    Object.defineProperty(this, 'stdout', { value: stdout, enumerable: false });
   }
 }
 
 export type FailureKind = 'not-a-video' | 'unavailable' | 'transient';
 
 const NOT_A_VIDEO_PATTERNS = [
   /unsupported url/i,
-  // the extractor tag is required by each pattern: yt-dlp's f4m downloader emits an
-  // untagged "No media found" for a transient condition that must stay retryable
   /\[reddit\] .+: no media found/i,
   /\[instagram\] .+: there is no video in this post/i,
+  /extension \(.+\) is unusual and will be skipped/i,
 ];
+// The extractor raises the friendly message above only for a single post; a
+// carousel is a playlist, whose photo items each fall through to yt-dlp's
+// generic no-formats error, a verdict on the item rather than the post.
+const PHOTO_ITEM = /\[instagram\] .+: no video formats found/i;
 const PERMANENT_PATTERNS = [
   /unable to extract/i,
   /no video formats found/i,
@@ -95,12 +109,15 @@ const PERMANENT_PATTERNS = [
 
 export const classifyFailure = (e: unknown): FailureKind => {
   if (!(e instanceof YtdlpError) || e.signalled) return 'transient';
-  const errorLines = e.stderr
-    .split('\n')
-    .filter((line) => line.startsWith('ERROR:'));
-  if (errorLines.some((l) => NOT_A_VIDEO_PATTERNS.some((re) => re.test(l))))
+  const all = errorLines(e.stderr);
+  if (!all.length) return 'transient';
+  // one line is the whole post reporting no formats, a failure the chat hears
+  // about
+  const rest = all.length > 1 ? all.filter((l) => !PHOTO_ITEM.test(l)) : all;
+  if (!rest.length) return 'not-a-video';
+  if (rest.some((l) => NOT_A_VIDEO_PATTERNS.some((re) => re.test(l))))
     return 'not-a-video';
-  if (errorLines.some((l) => PERMANENT_PATTERNS.some((re) => re.test(l))))
+  if (rest.some((l) => PERMANENT_PATTERNS.some((re) => re.test(l))))
     return 'unavailable';
   return 'transient';
 };
@@ -412,17 +429,20 @@ const execYtdlp = limit(
     // our own shutdown kill, not a failure: the queue leaves the job for the
     // next boot (a timeout kill sets no flag and stays a retryable YtdlpError)
     if (shuttingDown && proc.signalCode != null) throw new ShutdownAbort();
+
+    // stdout is read last, after stderr is fully drained: Bun buffers a piped
+    // child's stdout, so draining stderr to EOF first can't deadlock on a full
+    // stdout pipe (it would if stdout were unbuffered and left unread)
+    const stdout = await Bun.readableStreamToText(proc.stdout);
     if (proc.exitCode !== 0)
       throw new YtdlpError(
         getErrorMessage(proc),
         stderr,
         proc.signalCode != null,
+        stdout,
       );
 
-    // stdout is read last, after stderr is fully drained: Bun buffers a piped
-    // child's stdout, so draining stderr to EOF first can't deadlock on a full
-    // stdout pipe (it would if stdout were unbuffered and left unread)
-    return await Bun.readableStreamToText(proc.stdout);
+    return stdout;
   },
 );
 
@@ -451,58 +471,116 @@ const sweepInfoStmt = db.query<null, [number]>(
 export const sweepStaleInfo = () =>
   sweepInfoStmt.run(Date.now() - INFO_TTL_MS);
 
+// a playlist or channel URL resolves to the same multi-entry shape as a post,
+// but runs to thousands of entries
+export const MAX_POST_VIDEOS = 10;
+// yt-dlp spends a playlist index on a carousel's photos too, so a bound at the
+// delivery cap would hide videos behind them
+const MAX_SCRAPE_ITEMS = 50;
+
+const onlyPhotoItemsFailed = (e: YtdlpError) => {
+  const lines = errorLines(e.stderr);
+  // no ERROR line at all means the run failed for a reason this stderr no
+  // longer holds: the tail trim drops the oldest lines
+  return !!lines.length && lines.every((line) => PHOTO_ITEM.test(line));
+};
+
+const parseEntries = (out: string): VideoInfo[] =>
+  out
+    .split('\n')
+    .filter((line) => line.startsWith('{'))
+    .map((line) => JSON.parse(line) as VideoInfo);
+
 // Coalesced, not memoized: the DB row above serves repeat lookups, so keeping
 // settled results in memory would only pin megabytes of dump-json per URL and
 // grow stale (see INFO_TTL_MS); the in-flight entry alone stops two concurrent
 // jobs from both scraping.
-export const getInfo = coalesce(
+export const getInfos = coalesce(
   async (
     log: LogMessage,
     url: string,
     verbose: boolean = false,
-  ): Promise<VideoInfo> => {
+  ): Promise<VideoInfo[]> => {
     // a verbose request bypasses the cache (like the coalesce key below) so its
     // yt-dlp output is actually streamed to the chat for debugging
     const cached = selectInfoStmt.get(url, Date.now() - INFO_TTL_MS);
     if (cached && !verbose) return JSON.parse(cached.info);
 
     log.append(`🧐 <b>Scraping</b> ${url}...`);
 
-    const infoStr = await execYtdlp(log, url, verbose, '--dump-json');
-    const info = JSON.parse(infoStr) as VideoInfo;
-    const hadCanonical = !!info.webpage_url;
-    info.webpage_url ||= url;
-    // narrowed to a truthy string by the ||= above; a local const carries that
-    // through the tx closure (TS drops property narrowing across the boundary)
-    const canonical = info.webpage_url;
-    // also key a row by the canonical webpage_url so a later alias request
-    // for the same video hits the cache instead of re-scraping. Reuse
-    // yt-dlp's own string when nothing changed: re-serializing a multi-MB
-    // payload just to store identical content is wasted event-loop time.
-    const str = hadCanonical ? infoStr : JSON.stringify(info);
+    let out: string;
+    try {
+      out = await execYtdlp(
+        log,
+        url,
+        verbose,
+        '--dump-json',
+        // A link copied off a playing YouTube page carries the playlist it was
+        // playing in, and yt-dlp would take the playlist over the video.
+        // Instagram carousels are unaffected: they come back as a playlist
+        // whether or not this flag is set.
+        '--no-playlist',
+        '--playlist-end',
+        String(MAX_SCRAPE_ITEMS),
+      );
+    } catch (e) {
+      // A carousel mixing photos with videos fails on every photo item, so the
+      // run exits non-zero even though it resolved the videos we came for. A
+      // signal kill (a timeout) is different: its output stops mid-post, and
+      // possibly mid-line, so there is nothing trustworthy to salvage.
+      if (
+        !(e instanceof YtdlpError) ||
+        e.signalled ||
+        !e.stdout.trim() ||
+        !onlyPhotoItemsFailed(e)
+      ) {
+        throw e;
+      }
+      out = e.stdout;
+    }
+    const infos = parseEntries(out);
+    if (!infos.length) throw new Error('yt-dlp found no video at that URL');
+
+    for (const info of infos) info.webpage_url ||= url;
+    const canonical = infos[0]!.webpage_url!;
+    const namesAll = infos.every((i) => i.webpage_url === canonical);
+    const str = JSON.stringify(infos);
     const now = Date.now();
+    // A salvaged list that really did stop short is dropped by removeCachedUrl
+    // when a job cannot find its entry in it.
     tx(() => {
       insertInfoStmt.run(url, str, canonical, now);
-      if (canonical !== url) {
+      if (namesAll && canonical !== url) {
         insertInfoStmt.run(canonical, str, canonical, now);
       }
     });
-    return info;
+    return infos;
   },
   (_log, url, verbose) => !verbose && url,
 );
 
+export const getInfo = async (
+  log: LogMessage,
+  url: string,
+  verbose: boolean = false,
+): Promise<VideoInfo> => (await getInfos(log, url, verbose))[0]!;
+
 // Evict a video's cached info (the url row and its canonical alias share one
 // webpage_url) after a download failure: the likely cause is the expired signed
 // URLs above, so on the retry (or the next request) getInfo must re-scrape
 // rather than replay the same doomed row.
-const deleteInfoStmt = db.query<null, [string]>(
-  `DELETE FROM video_info WHERE webpage_url = ?`,
+const deleteInfoStmt = db.query<null, [string, string]>(
+  `DELETE FROM video_info WHERE webpage_url = ? OR url = ?`,
 );
-export const removeCachedInfo = (info: VideoInfo) => {
-  if (info.webpage_url) deleteInfoStmt.run(info.webpage_url);
+// `url` is the request that produced the row: a playlist's entries carry their
+// own webpage_url, so the entry alone cannot name the row it came from.
+export const removeCachedInfo = (info: VideoInfo, url?: string) => {
+  const key = info.webpage_url || url;
+  if (key) deleteInfoStmt.run(key, url ?? key);
 };
 
+export const removeCachedUrl = (url: string) => deleteInfoStmt.run(url, url);
+
 const logFormats = ({ formats }: any) =>
   // log all formats for debugging purposes
   formats &&