Skip to content

Commit 5c2b49d

Browse files
committed
fix(session): classify transport and timeout errors as retryable
MessageV2.fromError previously only treated ECONNRESET as retryable and classified everything else (including transient transport failures) as a hard error. Broaden it so a network blip recovers instead of failing the session: - transient SystemError codes (ECONNRESET, ETIMEDOUT, ENOTFOUND, EAI_AGAIN, ECONNREFUSED, EHOSTUNREACH, ENETUNREACH, EPIPE) - DOMException TimeoutError (AbortSignal.timeout) - bare network-error Error messages (fetch failed, socket hang up, SSE read timed out, terminated, other side closed, ...) A bare AbortError stays a cancel (classified by error identity, not an external flag), so user cancellation is never retried. Verified with unit tests in message-v2.test.ts covering each code, message pattern, and the abort/timeout distinction.
1 parent 2e71292 commit 5c2b49d

2 files changed

Lines changed: 113 additions & 8 deletions

File tree

packages/opencode/src/session/message-v2.ts

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -611,17 +611,52 @@ export function latest(msgs: WithParts[]) {
611611
return { user, assistant, finished, tasks }
612612
}
613613

614+
// System-level error codes that mean the transport failed in a way that's
615+
// safe to retry. ECONNRESET is by far the most common one, but a wifi blip
616+
// or DNS hiccup can produce any of these and they're all transient.
617+
const TRANSIENT_SYS_CODES = new Set([
618+
"ECONNRESET",
619+
"ECONNREFUSED",
620+
"ETIMEDOUT",
621+
"EAI_AGAIN",
622+
"ENOTFOUND",
623+
"EHOSTUNREACH",
624+
"ENETUNREACH",
625+
"EPIPE",
626+
])
627+
628+
// Message-level patterns produced by fetch / undici / our SSE chunk-timeout
629+
// wrapper when the transport fails without a SystemError code. These come
630+
// through as bare Errors and the only signal we have is the message text.
631+
const TRANSIENT_MSG_RE =
632+
/fetch failed|Failed to fetch|socket hang up|SSE read timed out|network ?error|Network request failed|other side closed|terminated|connect( |_)?error/i
633+
614634
export function fromError(
615635
e: unknown,
616636
ctx: { providerID: ProviderV2.ID; aborted?: boolean },
617637
): NonNullable<Assistant["error"]> {
638+
const sysCode = (e as SystemError)?.code
618639
switch (true) {
619640
case e instanceof DOMException && e.name === "AbortError":
620-
return new AbortedError(
621-
{ message: e.message },
622-
{
623-
cause: e,
624-
},
641+
// A *bare* AbortError only ever originates from `controller.abort()`
642+
// with no reason — i.e. a user/parent cancel (see provider.ts fetch
643+
// wrapper + processor onInterrupt). Every transport timeout we control
644+
// aborts with a *specific* reason and therefore surfaces as its own
645+
// identifiable error instead: AbortSignal.timeout() -> "TimeoutError"
646+
// DOMException (next case), the header-timeout -> HeaderTimeoutError,
647+
// and the SSE chunk-timeout -> ResponseStreamError ("SSE read timed
648+
// out"). Those are all classified retryable on their own below, so we
649+
// do NOT need to second-guess a bare AbortError here. Classifying by
650+
// error identity (rather than the externally-set `ctx.aborted` flag,
651+
// which races the abort propagating through the failure channel) keeps
652+
// a genuine cancel from being retried.
653+
return new AbortedError({ message: e.message }, { cause: e }).toObject()
654+
case e instanceof DOMException && e.name === "TimeoutError":
655+
// Modern fetch surfaces AbortSignal.timeout() as a DOMException with
656+
// name "TimeoutError" rather than "AbortError". Always retryable.
657+
return new APIError(
658+
{ message: e.message || "Request timed out", isRetryable: true, metadata: { code: "TimeoutError" } },
659+
{ cause: e },
625660
).toObject()
626661
case OutputLengthError.isInstance(e):
627662
return e
@@ -633,13 +668,13 @@ export function fromError(
633668
},
634669
{ cause: e },
635670
).toObject()
636-
case (e as SystemError)?.code === "ECONNRESET":
671+
case typeof sysCode === "string" && TRANSIENT_SYS_CODES.has(sysCode):
637672
return new APIError(
638673
{
639-
message: "Connection reset by server",
674+
message: sysCode === "ECONNRESET" ? "Connection reset by server" : `Network error (${sysCode})`,
640675
isRetryable: true,
641676
metadata: {
642-
code: (e as SystemError).code ?? "",
677+
code: sysCode ?? "",
643678
syscall: (e as SystemError).syscall ?? "",
644679
message: (e as SystemError).message ?? "",
645680
},
@@ -710,6 +745,15 @@ export function fromError(
710745
},
711746
{ cause: e },
712747
).toObject()
748+
case e instanceof Error && TRANSIENT_MSG_RE.test(e.message):
749+
// Bare Error from fetch/undici/wrapSSE where no SystemError code or
750+
// structured fields are attached. Sniffing the message is the best we
751+
// can do; getting these into SessionRetry.retryable lets a session
752+
// recover from a network blip instead of hard-failing.
753+
return new APIError(
754+
{ message: e.message, isRetryable: true, metadata: { code: "NETWORK_ERROR", message: e.message } },
755+
{ cause: e },
756+
).toObject()
713757
case e instanceof Error:
714758
return new NamedError.Unknown({ message: errorMessage(e) }, { cause: e }).toObject()
715759
default:

packages/opencode/test/session/message-v2.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,67 @@ describe("session.message-v2.fromError", () => {
15511551

15521552
expect(result.name).toBe("MessageAbortedError")
15531553
})
1554+
1555+
test("classifies user-cancelled AbortError as AbortedError when ctx.aborted is true", () => {
1556+
const err = new DOMException("Aborted", "AbortError")
1557+
const result = MessageV2.fromError(err, { providerID, aborted: true })
1558+
expect(result.name).toBe("MessageAbortedError")
1559+
})
1560+
1561+
test("classifies a bare AbortError as a cancel regardless of ctx.aborted", () => {
1562+
// A bare AbortError only comes from controller.abort() with no reason —
1563+
// i.e. a user/parent cancel. Transport timeouts surface as their own
1564+
// specific errors (TimeoutError/HeaderTimeoutError/ResponseStreamError),
1565+
// so a bare AbortError must NOT be reclassified as retryable — otherwise
1566+
// a cancel races the failure channel and gets retried.
1567+
const err = new DOMException("The operation was aborted", "AbortError")
1568+
const result = MessageV2.fromError(err, { providerID })
1569+
expect(result.name).toBe("MessageAbortedError")
1570+
})
1571+
1572+
test("classifies TimeoutError DOMException as retryable APIError", () => {
1573+
// AbortSignal.timeout() in modern fetch surfaces as TimeoutError.
1574+
const err = new DOMException("The operation timed out", "TimeoutError")
1575+
const result = MessageV2.fromError(err, { providerID })
1576+
expect(SessionV1.APIError.isInstance(result)).toBe(true)
1577+
expect((result as SessionV1.APIError).data.isRetryable).toBe(true)
1578+
})
1579+
1580+
test.each([
1581+
["ETIMEDOUT", "Network error (ETIMEDOUT)"],
1582+
["ENOTFOUND", "Network error (ENOTFOUND)"],
1583+
["EAI_AGAIN", "Network error (EAI_AGAIN)"],
1584+
["ECONNREFUSED", "Network error (ECONNREFUSED)"],
1585+
["EHOSTUNREACH", "Network error (EHOSTUNREACH)"],
1586+
["ENETUNREACH", "Network error (ENETUNREACH)"],
1587+
["EPIPE", "Network error (EPIPE)"],
1588+
])("classifies %s SystemError as retryable APIError", (code, expectedMessage) => {
1589+
const err = Object.assign(new Error(`${code} from test`), { code })
1590+
const result = MessageV2.fromError(err, { providerID })
1591+
expect(SessionV1.APIError.isInstance(result)).toBe(true)
1592+
expect((result as SessionV1.APIError).data.isRetryable).toBe(true)
1593+
expect((result as SessionV1.APIError).data.message).toBe(expectedMessage)
1594+
})
1595+
1596+
test.each([
1597+
"fetch failed",
1598+
"Failed to fetch",
1599+
"socket hang up",
1600+
"SSE read timed out",
1601+
"network error",
1602+
"Network request failed",
1603+
"other side closed",
1604+
"terminated",
1605+
])("classifies '%s' bare Error message as retryable APIError", (message) => {
1606+
const result = MessageV2.fromError(new Error(message), { providerID })
1607+
expect(SessionV1.APIError.isInstance(result)).toBe(true)
1608+
expect((result as SessionV1.APIError).data.isRetryable).toBe(true)
1609+
})
1610+
1611+
test("leaves unrelated Error messages classified as Unknown", () => {
1612+
const result = MessageV2.fromError(new Error("Some unrelated bug"), { providerID })
1613+
expect(result.name).toBe("UnknownError")
1614+
})
15541615
})
15551616

15561617
describe("session.message-v2.latest", () => {

0 commit comments

Comments
 (0)