Group chats: report why a promised video never arrived

src/download-video.ts

@@ -68,43 +68,51 @@ export class YtdlpError extends Error {
   }
 }
 
-// Failures a retry can't fix: the URL/extractor genuinely can't be handled. We
-// assume the background updater keeps yt-dlp current (see updateYtdlp), so a
-// fresh yt-dlp that still can't extract a URL means it's unsupported, not stale.
-// Anything not listed (network blips, 5xx, 408/429, a transient fragment
-// 404/403, unknown errors) is retryable: better to retry a lost cause a few
-// times than drop a video a retry would have delivered. The one HTTP
-// exception is a 403/404/410 on the *webpage* fetch: the URL is gone or the
-// site refuses us outright, and neither changes within the retry window
-// (a mid-download segment error isn't; that's why this is scoped to the webpage).
-const PERMANENT_PATTERNS = [
+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,
+];
+const PERMANENT_PATTERNS = [
   /unable to extract/i,
   /no video formats found/i,
   /is not a valid url/i,
   /private video/i,
   /video unavailable/i,
+  /empty media response/i,
   /no longer available/i,
   /has been removed/i,
   /members[- ]only/i,
   /sign in to confirm your age/i,
+  // scoped to the *webpage* fetch: a webpage 403/404/410 means the URL is gone
+  // or the site refuses us, but a mid-download segment 403 stays transient
   /unable to download webpage: http error (403|404|410)\b/i,
 ];
-// A signal kill (timeout/OOM) is always transient. For yt-dlp, match only its
-// own `ERROR:` lines, not WARNINGs or echoed page text, which can contain the
-// same phrases and would false-positive a retryable failure.
-export const isPermanentError = (e: unknown): boolean => {
+
+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))))
+    return 'not-a-video';
+  if (errorLines.some((l) => PERMANENT_PATTERNS.some((re) => re.test(l))))
+    return 'unavailable';
+  return 'transient';
+};
+
+// classifyFailure judges the video; Telegram errors are chat-level (blocked or
+// gone), so they are handled here rather than there.
+export const isPermanentError = (
+  e: unknown,
+  kind: FailureKind = classifyFailure(e),
+): boolean => {
   if (e instanceof YtdlpError) {
-    return (
-      !e.signalled &&
-      e.stderr
-        .split('\n')
-        .some(
-          (line) =>
-            line.startsWith('ERROR:') &&
-            PERMANENT_PATTERNS.some((re) => re.test(line)),
-        )
-    );
+    return kind !== 'transient';
   }
   // Telegram 403 = the user blocked the bot, or it was kicked/deactivated; a
   // few 400s name a gone chat/peer/reply-target. All are permanent-by-policy:
@@ -389,7 +397,7 @@ const execYtdlp = limit(
         stderr += text;
         if (stderr.length > 2 * STDERR_TAIL) {
           // trim on a line boundary so the cut never decapitates the `ERROR:`
-          // prefix that isPermanentError keys on
+          // prefix that classifyFailure keys on
           const nl = stderr.indexOf('\n', stderr.length - STDERR_TAIL);
           stderr = nl === -1 ? stderr.slice(-STDERR_TAIL) : stderr.slice(nl + 1);
         }

src/handlers.ts

@@ -10,6 +10,7 @@ import {
 import { db } from './db';
 import {
   calcDuration,
+  classifyFailure,
   downloadVideo,
   getInfo,
   isDownloaded,
@@ -21,6 +22,7 @@ import {
   tooLargeMessage,
   tooLargeToSend,
   YtdlpError,
+  type FailureKind,
   type VideoInfo,
 } from './download-video';
 import {
@@ -192,6 +194,23 @@ export const processJob = async (
     ? processUrlJob(telegram, job, attempt)
     : processConfirmedJob(telegram, job, attempt);
 
+// a link to one of these is always an explicit ask, so a terminal failure earns
+// its reason even in a group
+const ALWAYS_RESPOND_HOSTS = ['instagram.com', 'reddit.com', 'redd.it'];
+const isAlwaysRespondHost = (url: string): boolean => {
+  let host: string;
+  try {
+    // strip one trailing dot: a root-dot FQDN (reddit.com.) is the same host
+    host = new URL(url).hostname.replace(/\.$/, '');
+  } catch {
+    return false; // an unparseable URL is simply not whitelisted
+  }
+  return ALWAYS_RESPOND_HOSTS.some((h) => host === h || host.endsWith('.' + h));
+};
+
+const isTerminal = (e: unknown, attempt: number, kind?: FailureKind) =>
+  isPermanentError(e, kind) || attempt >= MAX_ATTEMPTS;
+
 const processUrlJob = async (
   telegram: Telegram,
   job: UrlJob,
@@ -301,7 +320,26 @@ const processUrlJob = async (
     // yt-dlp failures: after a send failure the info is fine, and keeping it
     // guarantees the retry maps to the same blob key and reuses the bytes.
     if (info && e instanceof YtdlpError) removeCachedInfo(info);
-    await reportJobFailure(job, log, e, attempt, '\n');
+    const kind = classifyFailure(e);
+    const terminal = isTerminal(e, attempt, kind);
+    // product policy: a not-a-video post (photo/article) never draws a group
+    // reply, whatever the host
+    const tellGroup =
+      chatType !== 'private' &&
+      terminal &&
+      kind !== 'not-a-video' &&
+      (info != null || isAlwaysRespondHost(url));
+    const reportLog = tellGroup
+      ? new LogMessage(telegram, logDestFor(job))
+      : log;
+    await reportJobFailure(
+      job,
+      reportLog,
+      e,
+      attempt,
+      terminal,
+      tellGroup ? '' : '\n',
+    );
     // reached only on a terminal failure (reportJobFailure rethrows retryable
     // ones, whose retry reuses the blob; a parked confirmation returned above).
     // Release the bytes this dead job downloaded.
@@ -369,7 +407,8 @@ const processConfirmedJob = async (
     // evict likely-expired cached info so the NEXT request re-scrapes; this
     // job's own retries can't benefit (the payload pins its info snapshot)
     if (e instanceof YtdlpError) removeCachedInfo(info);
-    await reportJobFailure(job, report(), e, attempt);
+    const terminal = isTerminal(e, attempt);
+    await reportJobFailure(job, report(), e, attempt, terminal);
     // terminal failure (retryable ones rethrew above and will reuse the blob):
     // release what this dead job owns
     await releaseAbandoned(info);
@@ -390,10 +429,11 @@ const reportJobFailure = async (
   log: LogMessage,
   e: any,
   attempt: number,
+  terminal: boolean,
   prefix = '',
 ) => {
   console.error(e); // log first: reporting to the user can itself fail
-  const retry = !isPermanentError(e) && attempt < MAX_ATTEMPTS;
+  const retry = !terminal;
   // The retry notice skips the reason (the streamed stderr above already shows
   // it, and the run isn't over) and trails a blank line to set off the next
   // attempt; the terminal report carries the reason. Groups see only that

src/log-message.ts

@@ -305,11 +305,8 @@ export class NoLog extends LogMessage {
   async flush() {}
 }
 
-// The group-silence policy, in one place: url jobs log progress and failures to
-// private chats only; a group would be spammed for every link anyone posts,
-// so group destinations get a NoLog. (Confirmed jobs are the deliberate
-// exception: an explicit confirm earns a group reply, so processConfirmedJob
-// constructs its report LogMessage directly.)
+// url-job progress logs to private chats only: a group would be spammed for
+// every link posted. Terminal failure reports come from their own call sites.
 export const logFor = (
   telegram: Telegram,
   chatType: string,

test/download-video.test.ts

@@ -42,6 +42,7 @@ import {
 } from './test-utils';
 import {
   abortDownloads,
+  classifyFailure,
   downloadVideo,
   getInfo,
   isPermanentError,
@@ -841,6 +842,67 @@ describe('isPermanentError', () => {
   });
 });
 
+describe('classifyFailure', () => {
+  it('classifies an Instagram empty-media response as unavailable', () => {
+    expect(
+      classifyFailure(
+        new YtdlpError(
+          'failed',
+          'ERROR: [Instagram] Da4FbMds5BU: Instagram sent an empty media response. Check if this post is accessible in your browser without being logged-in.',
+        ),
+      ),
+    ).toBe('unavailable');
+  });
+
+  it.each([
+    'ERROR: Unsupported URL: https://example.com/article',
+    'ERROR: [Reddit] 92dd8: No media found',
+    'ERROR: [Instagram] DbHhjdBJT9O: There is no video in this post',
+  ])('classifies %j as not-a-video', (stderr) => {
+    expect(classifyFailure(new YtdlpError('failed', stderr))).toBe(
+      'not-a-video',
+    );
+  });
+
+  it('keeps the f4m downloader\'s untagged "No media found" retryable', () => {
+    expect(
+      classifyFailure(new YtdlpError('failed', 'ERROR: No media found')),
+    ).toBe('transient');
+  });
+
+  it.each([
+    'ERROR: Private video. Sign in if you have access',
+    'ERROR: Unable to download webpage: HTTP Error 410: Gone',
+  ])('classifies %j as unavailable', (stderr) => {
+    expect(classifyFailure(new YtdlpError('failed', stderr))).toBe('unavailable');
+  });
+
+  it('not-a-video wins over unavailable when both patterns are present', () => {
+    const stderr =
+      'ERROR: Video unavailable\nERROR: Unsupported URL: https://x';
+    expect(classifyFailure(new YtdlpError('failed', stderr))).toBe(
+      'not-a-video',
+    );
+  });
+
+  it('classifies an Instagram rate-limit as transient (not unavailable)', () => {
+    expect(
+      classifyFailure(
+        new YtdlpError(
+          'failed',
+          'ERROR: [Instagram] xyz: Requested content is not available, rate-limit reached or login required',
+        ),
+      ),
+    ).toBe('transient');
+  });
+
+  it('diverges from isPermanentError on a Telegram permanent error', () => {
+    const e = telegramError(403, 'Forbidden: bot was blocked by the user');
+    expect(classifyFailure(e)).toBe('transient');
+    expect(isPermanentError(e)).toBe(true);
+  });
+});
+
 describe('sendVideo', () => {
   const cachedFileId = () => getBlob(VideoInfo)?.file_id;
 

test/e2e.test.ts

@@ -28,6 +28,9 @@ spyMock(console, 'table');
 
 const hiMessage = { text: 'hi' };
 
+const isFailureReport = (m: { text?: string }) =>
+  !!m.text && m.text.includes('💥 <b>Download failed</b>:');
+
 const urlMessage = (url: string, verbose?: boolean) => ({
   text: verbose ? `/verbose ${url}` : url,
   entities: [
@@ -100,9 +103,7 @@ describe.if(!!Bun.env.TEST_E2E)('message handler', async () => {
           waitUntil(
             () =>
               api.sentMessages.length > 1 ||
-              api.sentMessages.some(({ text }) =>
-                text?.includes('💥 <b>Download failed</b>:'),
-              ),
+              api.sentMessages.some(isFailureReport),
             ms,
           );
 
@@ -127,6 +128,58 @@ describe.if(!!Bun.env.TEST_E2E)('message handler', async () => {
       }),
     40_000,
   );
+
+  const groupChat = { id: -1000000000001, title: 'Test Group', type: 'supergroup' };
+
+  it(
+    'stays silent for a not-a-video link in a group',
+    () =>
+      withBotApi(async (api) => {
+        clearInMemoryCache();
+        api.sendTextMessageToBot(
+          urlMessage('https://www.instagram.com/p/DbHhjdBJT9O/'),
+          groupChat,
+        );
+        // gate on the job actually starting, else a never-started job passes this
+        // silence test vacuously
+        expect(await waitUntil(() => !jobsIdle(), 15_000)).toBe(true);
+        await waitUntil(jobsIdle, 25_000);
+        // a rate-limited scrape classifies 'unavailable' (whitelisted host =>
+        // one terminal report), so tolerate that live-scrape degradation the
+        // same way the download tests tolerate a 💥
+        if (api.sentMessages.length) {
+          const reports = api.sentMessages.filter(isFailureReport);
+          expect(reports).toHaveLength(1);
+          // pin the tolerated report to rate-limit/login wording: a broken
+          // not-a-video gate would instead report "There is no video in this
+          // post" and must still fail this test
+          expect(reports[0]!.text).toMatch(/rate.?limit|login required|empty media response/i);
+        } else {
+          expect(api.sentMessages).toEqual([]);
+        }
+      }),
+    45_000,
+  );
+
+  it(
+    'reports one 💥 for a whitelisted failing link in a group',
+    () =>
+      withBotApi(async (api) => {
+        setRetryBaseMs(1); // don't sleep the real backoff between attempts
+        clearInMemoryCache();
+        api.sendTextMessageToBot(
+          urlMessage('https://www.instagram.com/reel/C0aaaaaaaaa/'),
+          groupChat,
+        );
+        await waitUntil(() => api.sentMessages.some(isFailureReport), 60_000);
+        const reports = api.sentMessages.filter(isFailureReport);
+        expect(reports).toHaveLength(1);
+        expect(reports[0]!.chat_id).toBe(groupChat.id);
+        // id 0 is the link message: the first update in this fresh api
+        expect((reports[0] as any).reply_parameters?.message_id).toBe(0);
+      }),
+    90_000,
+  );
 });
 
 describe.todo('inline query handler');