diff --git a/apps/memos-local-plugin/core/embedding/fetcher.ts b/apps/memos-local-plugin/core/embedding/fetcher.ts index 303dae28e..142de13a8 100644 --- a/apps/memos-local-plugin/core/embedding/fetcher.ts +++ b/apps/memos-local-plugin/core/embedding/fetcher.ts @@ -17,6 +17,7 @@ import { type RetryPlan, waitForRetry, } from "../util/retry-after.js"; +import { isTransientNetworkError } from "../util/network-error.js"; import type { EmbeddingProviderName, ProviderLogger } from "./types.js"; export interface HttpPostOpts { @@ -150,7 +151,7 @@ export async function httpPostJson(opts: HttpPostOpts): Promise< } catch (err) { lastErr = err; if (err instanceof MemosError) throw err; - const transient = isTransientError(err); + const transient = isTransientNetworkError(err); opts.log.warn("http.exception", { url: opts.url, attempt, @@ -205,15 +206,9 @@ async function safeText(resp: Response): Promise { } } -function isTransientError(err: unknown): boolean { - if (!(err instanceof Error)) return false; - // Node fetch maps network errors to specific causes; abort with timeout is - // also retriable once. We're conservative here. - const msg = err.message ?? ""; - if (/timeout|ETIMEDOUT|ECONNRESET|EAI_AGAIN|socket hang up/i.test(msg)) return true; - if ((err as { code?: string }).code === "ABORT_ERR") return true; - return false; -} +// isTransientError moved to `core/util/network-error.ts` (isTransientNetworkError) +// so the embedding and LLM fetchers share one classification; it walks the +// undici `cause` chain instead of only the top-level message (issue #2379). function retryPlanDetails( plan: RetryPlan, diff --git a/apps/memos-local-plugin/core/llm/fetcher.ts b/apps/memos-local-plugin/core/llm/fetcher.ts index 358c73275..7f680f91d 100644 --- a/apps/memos-local-plugin/core/llm/fetcher.ts +++ b/apps/memos-local-plugin/core/llm/fetcher.ts @@ -20,6 +20,7 @@ import { type RetryPlan, waitForRetry, } from "../util/retry-after.js"; +import { isTransientNetworkError } from "../util/network-error.js"; import type { LlmProviderLogger, LlmProviderName } from "./types.js"; export interface HttpPostOpts { @@ -168,7 +169,7 @@ export async function httpPostJson(opts: HttpPostOpts): Promise< { provider: opts.provider, url: opts.url, cancelled: true }, ); } - const transient = isTransientError(err); + const transient = isTransientNetworkError(err); const timedOut = isTimeout(err) || opts.signal?.aborted === true; opts.log.warn("http.exception", { attempt, @@ -328,12 +329,9 @@ function truncateLogBody(text: string | undefined): string | undefined { return text?.slice(0, 512); } -function isTransientError(err: unknown): boolean { - if (!(err instanceof Error)) return false; - const msg = err.message ?? ""; - if (/ECONNRESET|EAI_AGAIN|socket hang up/i.test(msg)) return true; - return false; -} +// isTransientError moved to `core/util/network-error.ts` (isTransientNetworkError) +// so the LLM and embedding fetchers share one classification; it walks the +// undici `cause` chain instead of only the top-level message (issue #2379). function isTimeout(err: unknown): boolean { if (err instanceof Error) { diff --git a/apps/memos-local-plugin/core/util/network-error.ts b/apps/memos-local-plugin/core/util/network-error.ts new file mode 100644 index 000000000..fbbd13279 --- /dev/null +++ b/apps/memos-local-plugin/core/util/network-error.ts @@ -0,0 +1,45 @@ +/** + * Retryable network failure classification, shared by the LLM and embedding + * HTTP fetchers (previously two hand-copied `isTransientError` regexes). + * + * Node's built-in fetch (undici) surfaces *every* network-level failure as + * `TypeError: fetch failed` — the actual errno (`ECONNREFUSED`, `ECONNRESET`, + * …) only appears on `err.cause`. Classifying by the top-level message alone + * therefore never matches under undici, so real connectivity blips were + * logged as terminal (`transient=false`) and never retried + * (https://github.com/MemTensor/MemOS/issues/2379). + * + * We deliberately do NOT treat the bare `"fetch failed"` message as transient: + * TLS certificate failures and policy rejections carry the same message and + * must stay terminal. Classification keys off the cause chain's error *code*. + */ + +const TRANSIENT_MESSAGE_RE = /ECONNRESET|EAI_AGAIN|socket hang up|timeout|ETIMEDOUT/i; + +const TRANSIENT_CODE_RE = + /^(?:ECONNRESET|ECONNREFUSED|EAI_AGAIN|ENOTFOUND|ETIMEDOUT|EPIPE|UND_ERR|ABORT_ERR)$/i; + +/** Cause-chain hops to inspect before giving up (defensive cycle bound). */ +const MAX_CAUSE_DEPTH = 5; + +/** + * Whether `err` (or any of its `cause` links) looks like a retryable network + * failure. Walks undici's cause chain; undici nests the errno-carrying error + * under `TypeError: fetch failed`'s `cause`. + */ +export function isTransientNetworkError(err: unknown): boolean { + for ( + let cur: unknown = err, depth = 0; + cur instanceof Object && depth < MAX_CAUSE_DEPTH; + cur = (cur as { cause?: unknown }).cause, depth++ + ) { + const candidate = cur as { message?: unknown; code?: unknown }; + if (typeof candidate.message === "string" && TRANSIENT_MESSAGE_RE.test(candidate.message)) { + return true; + } + if (typeof candidate.code === "string" && TRANSIENT_CODE_RE.test(candidate.code)) { + return true; + } + } + return false; +} diff --git a/apps/memos-local-plugin/tests/unit/embedding/fetcher.test.ts b/apps/memos-local-plugin/tests/unit/embedding/fetcher.test.ts index 2811e94ef..6cf9aa127 100644 --- a/apps/memos-local-plugin/tests/unit/embedding/fetcher.test.ts +++ b/apps/memos-local-plugin/tests/unit/embedding/fetcher.test.ts @@ -219,6 +219,67 @@ describe("embedding/fetcher", () => { } }); + // Real undici shape: *every* network-level failure surfaces as + // `TypeError: fetch failed`, with the actual errno on `err.cause`. + // See https://github.com/MemTensor/MemOS/issues/2379 — these used to be + // classified transient=false and never retried. + it("retries a real undici network failure (fetch failed with ECONNREFUSED cause)", async () => { + const cause = Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:443"), { + code: "ECONNREFUSED", + }); + const f = mockFetch([ + new TypeError("fetch failed", { cause }), + new Response(JSON.stringify({ ok: 1 }), { status: 200 }), + ]); + + const res = await httpPostJson<{ ok: number }>({ + url: "https://x", + body: {}, + provider: "openai_compatible", + log: nullLogger(), + maxRetries: 1, + }); + expect(res).toMatchObject({ ok: 1 }); + expect(f).toHaveBeenCalledTimes(2); + }); + + it("retries a real undici network failure (fetch failed with ECONNRESET cause)", async () => { + const cause = Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" }); + const f = mockFetch([ + new TypeError("fetch failed", { cause }), + new Response(JSON.stringify({ ok: 1 }), { status: 200 }), + ]); + + const res = await httpPostJson<{ ok: number }>({ + url: "https://x", + body: {}, + provider: "mistral", + log: nullLogger(), + maxRetries: 1, + }); + expect(res).toMatchObject({ ok: 1 }); + expect(f).toHaveBeenCalledTimes(2); + }); + + it("does not retry a TLS certificate failure even though undici also reports fetch failed", async () => { + const cause = Object.assign( + new Error("Hostname/IP does not match certificate's altnames"), + { code: "ERR_TLS_CERT_ALTNAME_INVALID" }, + ); + const f = mockFetch([new TypeError("fetch failed", { cause })]); + + await expect( + httpPostJson({ + url: "https://x", + body: {}, + provider: "openai_compatible", + log: nullLogger(), + maxRetries: 2, + }), + ).rejects.toMatchObject({ code: "embedding_unavailable" }); + expect(f).toHaveBeenCalledTimes(1); + }); + it("merges an external AbortSignal", async () => { const ctrl = new AbortController(); mockFetch([ diff --git a/apps/memos-local-plugin/tests/unit/llm/fetcher.test.ts b/apps/memos-local-plugin/tests/unit/llm/fetcher.test.ts index 9b17e1b16..bf81555f4 100644 --- a/apps/memos-local-plugin/tests/unit/llm/fetcher.test.ts +++ b/apps/memos-local-plugin/tests/unit/llm/fetcher.test.ts @@ -362,6 +362,74 @@ describe("llm/fetcher", () => { } }); + // Real undici shape: *every* network-level failure surfaces as + // `TypeError: fetch failed`, with the actual errno on `err.cause`. + // See https://github.com/MemTensor/MemOS/issues/2379 — these used to be + // classified transient=false and never retried. + it("retries a real undici network failure (fetch failed with ECONNREFUSED cause)", async () => { + vi.useFakeTimers(); + const cause = Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:443"), { + code: "ECONNREFUSED", + }); + const f = mockFetch([ + new TypeError("fetch failed", { cause }), + new Response(JSON.stringify({ ok: 1 }), { status: 200 }), + ]); + + const pending = httpPostJson({ + url: "https://x", + body: {}, + timeoutMs: 5_000, + maxRetries: 1, + provider: "openai_compatible", + log: nullLog(), + }); + await vi.advanceTimersByTimeAsync(2_000); + await expect(pending).resolves.toMatchObject({ json: { ok: 1 } }); + expect(f).toHaveBeenCalledTimes(2); + }); + + it("retries a real undici network failure (fetch failed with ECONNRESET cause)", async () => { + vi.useFakeTimers(); + const cause = Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" }); + const f = mockFetch([ + new TypeError("fetch failed", { cause }), + new Response(JSON.stringify({ ok: 1 }), { status: 200 }), + ]); + + const pending = httpPostJson({ + url: "https://x", + body: {}, + timeoutMs: 5_000, + maxRetries: 1, + provider: "openai_compatible", + log: nullLog(), + }); + await vi.advanceTimersByTimeAsync(2_000); + await expect(pending).resolves.toMatchObject({ json: { ok: 1 } }); + expect(f).toHaveBeenCalledTimes(2); + }); + + it("does not retry a TLS certificate failure even though undici also reports fetch failed", async () => { + const cause = Object.assign( + new Error("Hostname/IP does not match certificate's altnames"), + { code: "ERR_TLS_CERT_ALTNAME_INVALID" }, + ); + const f = mockFetch([new TypeError("fetch failed", { cause })]); + + await expect( + httpPostJson({ + url: "https://x", + body: {}, + timeoutMs: 5_000, + maxRetries: 2, + provider: "openai_compatible", + log: nullLog(), + }), + ).rejects.toMatchObject({ code: "llm_unavailable" }); + expect(f).toHaveBeenCalledTimes(1); + }); + it("httpPostStream returns a ReadableStream body on 200", async () => { const body = new ReadableStream({ start(ctrl) {