Skip to content

Commit 44e88f5

Browse files
obviyusobviyus
committed
fix(telegram): keep media retry policy local
1 parent f9d7afd commit 44e88f5

6 files changed

Lines changed: 80 additions & 110 deletions

File tree

‎extensions/telegram/src/bot-handlers.media.test.ts‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,56 @@
1-
// Telegram tests cover inbound media-error classification for ingress retry.
21
import { MediaFetchError } from "openclaw/plugin-sdk/media-runtime";
32
import { describe, expect, it } from "vitest";
4-
import { isRecoverableMediaGroupError } from "./bot-handlers.media.js";
3+
import {
4+
isDurablyRetryableInboundMediaError,
5+
isRecoverableMediaGroupError,
6+
} from "./bot-handlers.media.js";
7+
8+
describe("isDurablyRetryableInboundMediaError", () => {
9+
const networkCause = () => Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" });
10+
const abortCause = () => Object.assign(new Error("aborted"), { name: "AbortError" });
11+
12+
it("retries transient network and shutdown abort fetch failures", () => {
13+
expect(
14+
isDurablyRetryableInboundMediaError(
15+
new MediaFetchError("fetch_failed", "x", { cause: networkCause() }),
16+
),
17+
).toBe(true);
18+
expect(
19+
isDurablyRetryableInboundMediaError(
20+
new MediaFetchError("fetch_failed", "x", { cause: abortCause() }),
21+
),
22+
).toBe(true);
23+
});
24+
25+
it("retries 408 and 5xx HTTP fetch failures", () => {
26+
for (const status of [408, 500, 502, 503, 504]) {
27+
expect(
28+
isDurablyRetryableInboundMediaError(new MediaFetchError("http_error", "x", { status })),
29+
).toBe(true);
30+
}
31+
});
32+
33+
it("does not retry permanent media failures", () => {
34+
expect(
35+
isDurablyRetryableInboundMediaError(
36+
new MediaFetchError("fetch_failed", "blocked: private address", {
37+
cause: new Error("blocked: private address"),
38+
}),
39+
),
40+
).toBe(false);
41+
for (const status of [400, 401, 403, 404, 429]) {
42+
expect(
43+
isDurablyRetryableInboundMediaError(new MediaFetchError("http_error", "x", { status })),
44+
).toBe(false);
45+
}
46+
expect(isDurablyRetryableInboundMediaError(new MediaFetchError("max_bytes", "too big"))).toBe(
47+
false,
48+
);
49+
});
50+
});
551

652
describe("isRecoverableMediaGroupError preserves album partial delivery (#55216)", () => {
753
it("still skips-and-warns transient and permanent album fetch failures", () => {
8-
// Media groups intentionally skip the unfetchable photo and warn (#55216);
9-
// only the single-message path durably retries transient failures (#98076,
10-
// via isDurablyRetryableMediaFetchError). Keep this path unchanged.
1154
expect(isRecoverableMediaGroupError(new MediaFetchError("fetch_failed", "x"))).toBe(true);
1255
expect(isRecoverableMediaGroupError(new MediaFetchError("max_bytes", "x"))).toBe(true);
1356
});

‎extensions/telegram/src/bot-handlers.media.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Telegram plugin module implements bot handlers.media behavior.
22
import type { Message } from "grammy/types";
33
import { MediaFetchError } from "openclaw/plugin-sdk/media-runtime";
4+
import { isRecoverableTelegramNetworkError } from "./network-errors.js";
45

56
export function isMediaSizeLimitError(err: unknown): boolean {
67
const errMsg = String(err);
@@ -11,6 +12,33 @@ export function isRecoverableMediaGroupError(err: unknown): boolean {
1112
return err instanceof MediaFetchError || isMediaSizeLimitError(err);
1213
}
1314

15+
function isAbortError(err: unknown): boolean {
16+
if (!err || typeof err !== "object") {
17+
return false;
18+
}
19+
if ("name" in err && err.name === "AbortError") {
20+
return true;
21+
}
22+
return "message" in err && err.message === "This operation was aborted";
23+
}
24+
25+
export function isDurablyRetryableInboundMediaError(err: unknown): boolean {
26+
if (!(err instanceof MediaFetchError)) {
27+
return false;
28+
}
29+
if (err.code === "http_error") {
30+
return typeof err.status === "number" && (err.status === 408 || err.status >= 500);
31+
}
32+
if (err.code !== "fetch_failed") {
33+
return false;
34+
}
35+
return (
36+
isAbortError(err) ||
37+
isAbortError(err.cause) ||
38+
isRecoverableTelegramNetworkError(err, { context: "polling" })
39+
);
40+
}
41+
1442
export function hasInboundMedia(msg: Message): boolean {
1543
return (
1644
Boolean(msg.media_group_id) ||

‎extensions/telegram/src/bot-handlers.runtime.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import {
3131
resolvePluginConversationBindingApproval,
3232
} from "openclaw/plugin-sdk/conversation-runtime";
3333
import { isApprovalNotFoundError } from "openclaw/plugin-sdk/error-runtime";
34-
import { isDurablyRetryableMediaFetchError } from "openclaw/plugin-sdk/media-runtime";
3534
import { applyModelOverrideToSessionEntry } from "openclaw/plugin-sdk/model-session-runtime";
3635
import { formatModelsAvailableHeader } from "openclaw/plugin-sdk/models-provider-runtime";
3736
import { parseStrictPositiveInteger } from "openclaw/plugin-sdk/number-runtime";
@@ -67,6 +66,7 @@ import {
6766
} from "./bot-handlers.debounce-key.js";
6867
import {
6968
hasInboundMedia,
69+
isDurablyRetryableInboundMediaError,
7070
isMediaSizeLimitError,
7171
isRecoverableMediaGroupError,
7272
resolveInboundMediaFileId,
@@ -2273,17 +2273,10 @@ export const registerTelegramHandlers = ({
22732273
return;
22742274
}
22752275
logger.warn({ chatId, error: String(mediaErr) }, "media fetch failed");
2276-
const retryable = isDurablyRetryableMediaFetchError(mediaErr);
2276+
const retryable = isDurablyRetryableInboundMediaError(mediaErr);
22772277
if (retryable) {
2278-
// Keep the durable spool entry so a transient/shutdown media fetch
2279-
// failure is re-driven after restart instead of acked and lost (#98076).
2280-
// bot-core keeps spooled replays (completed:false) and best-effort acks
2281-
// live updates on this failed-retryable result.
22822278
recordTelegramMessageProcessingResult({ kind: "failed-retryable", error: mediaErr });
22832279
}
2284-
// On spooled replay the update is durably re-driven, so suppress the
2285-
// user-facing retry notice that would otherwise repeat on each replay.
2286-
// Live updates and permanent failures still get the best-effort warning.
22872280
if (!(retryable && isTelegramSpooledReplayUpdate(ctx.update))) {
22882281
await withTelegramApiErrorLogging({
22892282
operation: "sendMessage",

‎extensions/telegram/src/bot.create-telegram-bot.channel-post-media.test.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ vi.mock("./bot/delivery.resolve-media.runtime.js", async () => {
3838
throw new actual.MediaFetchError(
3939
"fetch_failed",
4040
err instanceof Error ? err.message : String(err),
41+
{ cause: err },
4142
);
4243
}
4344
},
@@ -458,9 +459,7 @@ describe("createTelegramBot channel_post media", () => {
458459
});
459460
sendMessageSpy.mockClear();
460461
replySpy.mockClear();
461-
// A shutdown/abort mid-fetch (the primary restart-window loss vector) is
462-
// durably retryable even though the in-loop retry policy skips aborts.
463-
saveRemoteMedia.mockRejectedValue(new Error("This operation was aborted"));
462+
saveRemoteMedia.mockRejectedValue(Object.assign(new Error("aborted"), { name: "AbortError" }));
464463

465464
createTelegramBot({ token: "tok" });
466465
const handler = getOnHandler("message") as (ctx: Record<string, unknown>) => Promise<void>;
@@ -482,9 +481,6 @@ describe("createTelegramBot channel_post media", () => {
482481
withTelegramSpooledReplayUpdate(update, () => handler(ctx)),
483482
);
484483

485-
// Spooled replay keeps the update (completed:false) so the document is
486-
// re-downloaded after restart instead of acked and lost; the "try again"
487-
// notice is suppressed because we are durably retrying.
488484
expect(result).toEqual({ kind: "failed-retryable", error: expect.any(MediaFetchError) });
489485
expect(sendMessageSpy).not.toHaveBeenCalled();
490486
});
@@ -519,8 +515,6 @@ describe("createTelegramBot channel_post media", () => {
519515
withTelegramSpooledReplayUpdate(update, () => handler(ctx)),
520516
);
521517

522-
// A permanent failure never succeeds on replay, so it stays best-effort:
523-
// the update is acked (no failed-retryable result) and the user is warned.
524518
expect(result).toBeUndefined();
525519
await waitForMockCalls(sendMessageSpy, 1);
526520
expect(sendMessageSpy).toHaveBeenCalledWith(
@@ -538,9 +532,6 @@ describe("createTelegramBot channel_post media", () => {
538532
});
539533
sendMessageSpy.mockClear();
540534
replySpy.mockClear();
541-
// A non-network, non-abort fetch_failed (e.g. SSRF/guard denial or a local
542-
// Bot API path/read failure) must NOT be durably requeued — it would loop in
543-
// the spool with the warning suppressed. It stays best-effort: acked + warned.
544535
saveRemoteMedia.mockRejectedValue(new Error("blocked by SSRF guard: private address"));
545536

546537
createTelegramBot({ token: "tok" });

‎src/media/fetch.test.ts‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,65 +1015,3 @@ describe("readRemoteMediaBuffer", () => {
10151015
expect(fetchImpl).toHaveBeenCalledTimes(1);
10161016
});
10171017
});
1018-
1019-
describe("isDurablyRetryableMediaFetchError", () => {
1020-
let isDurablyRetryableMediaFetchError: FetchModule["isDurablyRetryableMediaFetchError"];
1021-
let MediaFetchError: FetchModule["MediaFetchError"];
1022-
1023-
beforeAll(async () => {
1024-
const mod = await import("./fetch.js");
1025-
isDurablyRetryableMediaFetchError = mod.isDurablyRetryableMediaFetchError;
1026-
MediaFetchError = mod.MediaFetchError;
1027-
});
1028-
1029-
const transientCause = () => Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" });
1030-
const abortCause = () => Object.assign(new Error("aborted"), { name: "AbortError" });
1031-
1032-
it("retries transient-network fetch failures", () => {
1033-
expect(
1034-
isDurablyRetryableMediaFetchError(
1035-
new MediaFetchError("fetch_failed", "x", { cause: transientCause() }),
1036-
),
1037-
).toBe(true);
1038-
});
1039-
1040-
it("retries shutdown/abort fetch failures the in-loop policy skips", () => {
1041-
expect(
1042-
isDurablyRetryableMediaFetchError(
1043-
new MediaFetchError("fetch_failed", "x", { cause: abortCause() }),
1044-
),
1045-
).toBe(true);
1046-
});
1047-
1048-
it("does not retry permanent fetch failures (SSRF/guard, local path)", () => {
1049-
expect(
1050-
isDurablyRetryableMediaFetchError(
1051-
new MediaFetchError("fetch_failed", "blocked: private address", {
1052-
cause: new Error("blocked: private address"),
1053-
}),
1054-
),
1055-
).toBe(false);
1056-
});
1057-
1058-
it("retries 408/5xx HTTP errors but not other 4xx or size limits", () => {
1059-
for (const status of [408, 500, 502, 503, 504]) {
1060-
expect(
1061-
isDurablyRetryableMediaFetchError(new MediaFetchError("http_error", "x", { status })),
1062-
).toBe(true);
1063-
}
1064-
for (const status of [400, 401, 403, 404, 429]) {
1065-
expect(
1066-
isDurablyRetryableMediaFetchError(new MediaFetchError("http_error", "x", { status })),
1067-
).toBe(false);
1068-
}
1069-
expect(isDurablyRetryableMediaFetchError(new MediaFetchError("max_bytes", "too big"))).toBe(
1070-
false,
1071-
);
1072-
});
1073-
1074-
it("does not retry non-network non-MediaFetchError values", () => {
1075-
expect(isDurablyRetryableMediaFetchError(new Error("boom"))).toBe(false);
1076-
expect(isDurablyRetryableMediaFetchError("nope")).toBe(false);
1077-
expect(isDurablyRetryableMediaFetchError(undefined)).toBe(false);
1078-
});
1079-
});

‎src/media/fetch.ts‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -550,29 +550,6 @@ function shouldRetryMediaFetch(err: unknown): boolean {
550550
return isTransientNetworkError(err);
551551
}
552552

553-
/**
554-
* Whether a media fetch failure should be retried by a durable ingress spool
555-
* after a restart (e.g. Telegram inbound spool replay), as opposed to the
556-
* in-fetch retry loop owned by {@link shouldRetryMediaFetch}.
557-
*
558-
* It retries everything the in-loop policy retries, plus shutdown/abort
559-
* `fetch_failed` errors: an in-loop retry mid-shutdown is futile, but a
560-
* restart-window abort is a primary inbound-media loss vector and is
561-
* recoverable on replay. Permanent failures (size limit, non-retryable HTTP,
562-
* and SSRF/guard or local-path `fetch_failed`) stay non-retryable so they do
563-
* not loop in the spool.
564-
*/
565-
export function isDurablyRetryableMediaFetchError(err: unknown): boolean {
566-
if (shouldRetryMediaFetch(err)) {
567-
return true;
568-
}
569-
return (
570-
err instanceof MediaFetchError &&
571-
err.code === "fetch_failed" &&
572-
(isAbortError(err) || isAbortError(err.cause))
573-
);
574-
}
575-
576553
async function withMediaFetchRetry<T>(
577554
options: FetchMediaOptions,
578555
fn: () => Promise<T>,

0 commit comments

Comments
 (0)