From e2c4993be7625ffcbc2167548578b44688e5bd02 Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Sat, 14 Mar 2026 18:52:25 +0800 Subject: [PATCH 1/3] fix(telegram): add retry logic to media file download --- .../bot/delivery.resolve-media-retry.test.ts | 62 ++++++++++++++++++- .../src/bot/delivery.resolve-media.ts | 56 +++++++++++++---- 2 files changed, 104 insertions(+), 14 deletions(-) diff --git a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts index 86d6e608dce..f54d62abe67 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts @@ -200,15 +200,71 @@ describe("resolveMedia getFile retry", () => { }, ); - it("does not catch errors from fetchRemoteMedia (only getFile is retried)", async () => { + it("retries fetchRemoteMedia on transient network failure and succeeds", async () => { const getFile = vi.fn().mockResolvedValue({ file_path: "voice/file_0.oga" }); - fetchRemoteMedia.mockRejectedValueOnce(new Error("download failed")); + fetchRemoteMedia + .mockRejectedValueOnce(new MockMediaFetchError("fetch_failed", "Network request failed")) + .mockResolvedValueOnce({ + buffer: Buffer.from("audio"), + contentType: "audio/ogg", + fileName: "file_0.oga", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/file_0.oga", + contentType: "audio/ogg", + }); + + const promise = resolveMedia(makeCtx("voice", getFile), MAX_MEDIA_BYTES, BOT_TOKEN); + await flushRetryTimers(); + const result = await promise; + + expect(getFile).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledTimes(2); + expect(result).toEqual( + expect.objectContaining({ path: "/tmp/file_0.oga", placeholder: "" }), + ); + }); + + it("does not retry fetchRemoteMedia on permanent http_error", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "voice/file_0.oga" }); + fetchRemoteMedia.mockRejectedValueOnce( + new MockMediaFetchError("http_error", "HTTP 404 Not Found"), + ); await expect( resolveMedia(makeCtx("voice", getFile), MAX_MEDIA_BYTES, BOT_TOKEN), - ).rejects.toThrow("download failed"); + ).rejects.toThrow("HTTP 404 Not Found"); expect(getFile).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); + }); + + it("does not retry fetchRemoteMedia on max_bytes policy violation", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "video/large.mp4" }); + fetchRemoteMedia.mockRejectedValueOnce( + new MockMediaFetchError("max_bytes", "File exceeds maximum size"), + ); + + await expect( + resolveMedia(makeCtx("video", getFile), MAX_MEDIA_BYTES, BOT_TOKEN), + ).rejects.toThrow("File exceeds maximum size"); + + expect(getFile).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); + }); + + it("throws after fetchRemoteMedia exhausts retries on persistent network failure", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "voice/file_0.oga" }); + fetchRemoteMedia.mockRejectedValue( + new MockMediaFetchError("fetch_failed", "Network request failed"), + ); + + const promise = resolveMedia(makeCtx("voice", getFile), MAX_MEDIA_BYTES, BOT_TOKEN); + await flushRetryTimers(); + + await expect(promise).rejects.toThrow("Network request failed"); + expect(getFile).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledTimes(3); }); it("does not retry 'file is too big' error (400 Bad Request) and returns null", async () => { diff --git a/extensions/telegram/src/bot/delivery.resolve-media.ts b/extensions/telegram/src/bot/delivery.resolve-media.ts index 2e552529dec..d096a1beb05 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { GrammyError } from "grammy"; import { formatErrorMessage } from "openclaw/plugin-sdk/infra-runtime"; import { retryAsync } from "openclaw/plugin-sdk/infra-runtime"; -import { fetchRemoteMedia } from "openclaw/plugin-sdk/media-runtime"; +import { fetchRemoteMedia, MediaFetchError } from "openclaw/plugin-sdk/media-runtime"; import { saveMediaBuffer } from "openclaw/plugin-sdk/media-runtime"; import { logVerbose, warn } from "openclaw/plugin-sdk/runtime-env"; import { @@ -60,6 +60,27 @@ function isRetryableGetFileError(err: unknown): boolean { return true; } +/** + * Returns true if the download error is a transient network error that should be retried. + * Returns false for permanent errors like HTTP 4xx, max_bytes policy violations. + */ +function isRetryableDownloadError(err: unknown): boolean { + if (err instanceof MediaFetchError) { + // Only retry transient fetch failures (network issues, timeouts, connection drops). + // Do not retry HTTP errors (4xx, 5xx) or policy violations (max_bytes). + return err.code === "fetch_failed"; + } + // For non-MediaFetchError, check if it looks like a transient network error. + const msg = formatErrorMessage(err).toLowerCase(); + return ( + msg.includes("fetch failed") || + msg.includes("network") || + msg.includes("econnreset") || + msg.includes("etimedout") || + msg.includes("econnrefused") + ); +} + function resolveMediaFileRef(msg: TelegramContext["message"]) { return ( msg.photo?.[msg.photo.length - 1] ?? @@ -149,16 +170,29 @@ async function downloadAndSaveTelegramFile(params: { } const apiBase = resolveTelegramApiBase(params.apiRoot); const url = `${apiBase}/file/bot${params.token}/${params.filePath}`; - const fetched = await fetchRemoteMedia({ - url, - fetchImpl: params.transport.sourceFetch, - dispatcherAttempts: params.transport.dispatcherAttempts, - shouldRetryFetchError: shouldRetryTelegramTransportFallback, - filePathHint: params.filePath, - maxBytes: params.maxBytes, - readIdleTimeoutMs: TELEGRAM_DOWNLOAD_IDLE_TIMEOUT_MS, - ssrfPolicy: buildTelegramMediaSsrfPolicy(params.apiRoot), - }); + const fetched = await retryAsync( + () => + fetchRemoteMedia({ + url, + fetchImpl: params.transport.sourceFetch, + dispatcherAttempts: params.transport.dispatcherAttempts, + shouldRetryFetchError: shouldRetryTelegramTransportFallback, + filePathHint: params.filePath, + maxBytes: params.maxBytes, + readIdleTimeoutMs: TELEGRAM_DOWNLOAD_IDLE_TIMEOUT_MS, + ssrfPolicy: buildTelegramMediaSsrfPolicy(params.apiRoot), + }), + { + attempts: 3, + minDelayMs: 500, + maxDelayMs: 2000, + jitter: 0.2, + label: "telegram:downloadFile", + shouldRetry: isRetryableDownloadError, + onRetry: ({ attempt, maxAttempts }) => + logVerbose(`telegram: file download retry ${attempt}/${maxAttempts}`), + }, + ); const originalName = params.telegramFileName ?? fetched.fileName ?? params.filePath; return saveMediaBuffer( fetched.buffer, From b54f823a039dc59e1a538269c101d5b55f2e9203 Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Sat, 14 Mar 2026 21:10:24 +0800 Subject: [PATCH 2/3] fix(telegram): retry transient 5xx download errors --- .../bot/delivery.resolve-media-retry.test.ts | 25 +++++++++++++++++++ .../src/bot/delivery.resolve-media.ts | 16 +++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts index f54d62abe67..e306ef2a605 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts @@ -239,6 +239,31 @@ describe("resolveMedia getFile retry", () => { expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); }); + it("retries fetchRemoteMedia on transient HTTP 5xx error and succeeds", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "voice/file_0.oga" }); + fetchRemoteMedia + .mockRejectedValueOnce(new MockMediaFetchError("http_error", "HTTP 502 Bad Gateway")) + .mockResolvedValueOnce({ + buffer: Buffer.from("audio"), + contentType: "audio/ogg", + fileName: "file_0.oga", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/file_0.oga", + contentType: "audio/ogg", + }); + + const promise = resolveMedia(makeCtx("voice", getFile), MAX_MEDIA_BYTES, BOT_TOKEN); + await flushRetryTimers(); + const result = await promise; + + expect(getFile).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledTimes(2); + expect(result).toEqual( + expect.objectContaining({ path: "/tmp/file_0.oga", placeholder: "" }), + ); + }); + it("does not retry fetchRemoteMedia on max_bytes policy violation", async () => { const getFile = vi.fn().mockResolvedValue({ file_path: "video/large.mp4" }); fetchRemoteMedia.mockRejectedValueOnce( diff --git a/extensions/telegram/src/bot/delivery.resolve-media.ts b/extensions/telegram/src/bot/delivery.resolve-media.ts index d096a1beb05..e0d744be096 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media.ts @@ -66,9 +66,19 @@ function isRetryableGetFileError(err: unknown): boolean { */ function isRetryableDownloadError(err: unknown): boolean { if (err instanceof MediaFetchError) { - // Only retry transient fetch failures (network issues, timeouts, connection drops). - // Do not retry HTTP errors (4xx, 5xx) or policy violations (max_bytes). - return err.code === "fetch_failed"; + // Retry transient fetch failures (network issues, timeouts, connection drops). + if (err.code === "fetch_failed") { + return true; + } + // Retry transient HTTP 5xx server errors; do not retry 4xx or policy violations. + if (err.code === "http_error") { + const match = /HTTP (\d{3})/.exec(err.message); + if (match) { + const status = Number(match[1]); + return status >= 500; + } + } + return false; } // For non-MediaFetchError, check if it looks like a transient network error. const msg = formatErrorMessage(err).toLowerCase(); From 37c8b3dc08a234e51c06f9737c8823d75db4a624 Mon Sep 17 00:00:00 2001 From: Jerry-Xin Date: Wed, 18 Mar 2026 03:10:44 +0800 Subject: [PATCH 3/3] fix(telegram): define MockMediaFetchError in retry test file Import the real MediaFetchError via vi.importActual and alias it as MockMediaFetchError so the retry test cases can instantiate typed errors that pass the instanceof check inside isRetryableDownloadError. --- .../src/bot/delivery.resolve-media-retry.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts index e306ef2a605..a1233fe4bf3 100644 --- a/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts +++ b/extensions/telegram/src/bot/delivery.resolve-media-retry.test.ts @@ -111,6 +111,16 @@ function makeCtx( }; } +/** + * Re-export the real MediaFetchError so tests throw instances that pass the + * `instanceof` check inside `isRetryableDownloadError`. The vi.mock above + * preserves the original class via `...actual`, so both sides reference the + * same constructor. + */ +// eslint-disable-next-line @typescript-eslint/no-require-imports -- dynamic import after mock registration +const { MediaFetchError: MockMediaFetchError } = + await vi.importActual("openclaw/plugin-sdk/media-runtime"); + function setupTransientGetFileRetry() { const getFile = vi .fn()