Skip to content

Commit 26dc319

Browse files
committed
refactor(provider, message-v2): unify SSEStallError on MessageV2 schema
Drop the duplicate `class SSEStallError extends Error` from provider.ts and have wrapSSE() throw `MessageV2.SSEStallError` (the schema-backed error) directly. Spike confirmed identity survives in-process rethrow paths (throw/catch, AbortController.abort, AbortSignal.any composition, ReadableStream.cancel, Promise.reject, end-to-end wrapSSE shape — 6/6). Kept hasSSEStallCause + SSE_STALL_MESSAGE_RE (hardened in F4) as the cross-realm safety net. The substring fallback defends rethrow paths that strip Error name/_tag (vendor SDK rewrapping, structured-clone boundaries) and is independent of which class is thrown. Added extractStallMessage() helper because the schema-error class sets `.message` to its tag (via `super(tag, options)`), so fromError can no longer copy `.message` directly when the input is already a schema instance — must read `.data.message`. Walks cause chain so nested wrappers still produce the original timing text. Updated test files to construct `MessageV2.SSEStallError` directly instead of the deleted runtime class. Net delta: -11 / +40 LOC. Eliminates one of two SSEStallError classes; remaining duplication is the substring fallback regex which intentionally defends a different threat model than the class identity. Addresses audit finding F9 (codex-5.3 diamond review, 2026-04-22).
1 parent c105c60 commit 26dc319

5 files changed

Lines changed: 50 additions & 19 deletions

File tree

packages/opencode/src/provider/provider.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,13 @@ import { withStatics } from "@/util/schema"
2929

3030
import * as ProviderTransform from "./transform"
3131
import { ModelID, ProviderID } from "./schema"
32+
// message-v2 imports ProviderError directly from @/provider/error (not the
33+
// @/provider barrel), so this back-edge does not form a cycle through
34+
// provider.ts. Do not change the import in message-v2.ts back to the barrel.
35+
import { MessageV2 } from "@/session/message-v2"
3236

3337
const log = Log.create({ service: "provider" })
3438

35-
export class SSEStallError extends Error {
36-
readonly _tag = "SSEStallError"
37-
constructor(message: string) {
38-
super(message)
39-
this.name = "SSEStallError"
40-
}
41-
}
42-
4339
function shouldUseCopilotResponsesApi(modelID: string): boolean {
4440
const match = /^gpt-(\d+)/.exec(modelID)
4541
if (!match) return false
@@ -77,7 +73,7 @@ export function wrapSSE(res: Response, ms: number, ctl: AbortController) {
7773
async pull(ctrl) {
7874
const part = await new Promise<Awaited<ReturnType<typeof reader.read>>>((resolve, reject) => {
7975
const id = setTimeout(() => {
80-
const err = new SSEStallError(`SSE read timed out after ${ms}ms`)
76+
const err = new MessageV2.SSEStallError({ message: `SSE read timed out after ${ms}ms` })
8177
ctl.abort(err)
8278
void reader.cancel(err)
8379
reject(err)

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import { Snapshot } from "@/snapshot"
88
import { SyncEvent } from "../sync"
99
import { Database, NotFoundError, and, desc, eq, inArray, lt, or } from "@/storage"
1010
import { MessageTable, PartTable, SessionTable } from "./session.sql"
11-
import { ProviderError } from "@/provider"
11+
// Import directly from provider/error (not the @/provider barrel) to avoid a
12+
// circular import: provider/provider.ts now imports MessageV2 from this file,
13+
// and the barrel re-exports provider.ts.
14+
import * as ProviderError from "@/provider/error"
1215
import { iife } from "@/util/iife"
1316
import { errorMessage } from "@/util/error"
1417
import { isMedia } from "@/util/media"
@@ -1092,6 +1095,30 @@ function hasSSEStallCause(e: unknown, depth = 0): boolean {
10921095
return false
10931096
}
10941097

1098+
// Extract the user-meaningful "SSE read timed out after Nms" string from an
1099+
// error that hasSSEStallCause matched. NamedSchemaError sets Error.prototype.message
1100+
// to the tag (`super(tag, options)` in named-schema-error.ts), so an in-process
1101+
// throw of MessageV2.SSEStallError caught and passed back through fromError must
1102+
// read `.data.message`, not `.message`, to recover the timing text. Plain Error
1103+
// instances (legacy or cross-realm) put it in `.message`. Walk the cause chain
1104+
// so nested wrappers still produce the original timing text.
1105+
function extractStallMessage(e: unknown, depth = 0): string {
1106+
if (depth > 8) return String(e)
1107+
if (!e || typeof e !== "object") return String(e)
1108+
const err = e as { data?: { message?: unknown }; message?: unknown; cause?: unknown }
1109+
if (err.data && typeof err.data.message === "string" && SSE_STALL_MESSAGE_RE.test(err.data.message)) {
1110+
return err.data.message
1111+
}
1112+
if (typeof err.message === "string" && SSE_STALL_MESSAGE_RE.test(err.message)) return err.message
1113+
if (err.cause) return extractStallMessage(err.cause, depth + 1)
1114+
// hasSSEStallCause already matched; produce best-effort text even if no node
1115+
// has the canonical "after Nms" timing format (e.g., test fixture passes
1116+
// "SSE read timed out" without the suffix).
1117+
if (err.data && typeof err.data.message === "string") return err.data.message
1118+
if (typeof err.message === "string") return err.message
1119+
return String(e)
1120+
}
1121+
10951122
export function fromError(
10961123
e: unknown,
10971124
ctx: { providerID: ProviderID; aborted?: boolean },
@@ -1144,7 +1171,7 @@ export function fromError(
11441171
).toObject()
11451172
case hasSSEStallCause(e):
11461173
return new SSEStallError(
1147-
{ message: e instanceof Error ? e.message : String(e) },
1174+
{ message: extractStallMessage(e) },
11481175
{ cause: e instanceof Error ? e : undefined },
11491176
).toObject()
11501177
case APICallError.isInstance(e):

packages/opencode/test/provider/chunk-timeout.test.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test } from "bun:test"
2-
import { resolveChunkTimeout, SSEStallError, wrapSSE } from "../../src/provider/provider"
2+
import { resolveChunkTimeout, wrapSSE } from "../../src/provider/provider"
3+
import { MessageV2 } from "../../src/session/message-v2"
34

45
describe("provider.resolveChunkTimeout", () => {
56
test("returns 120s default when undefined and reasoning=false", () => {
@@ -51,7 +52,7 @@ describe("provider.resolveChunkTimeout", () => {
5152
})
5253

5354
describe("provider.wrapSSE — SSEStallError integration", () => {
54-
test("throws SSEStallError when chunk read exceeds timeout", async () => {
55+
test("throws MessageV2.SSEStallError when chunk read exceeds timeout", async () => {
5556
const stream = new ReadableStream<Uint8Array>({
5657
pull() {
5758
return new Promise<void>(() => {}) // never resolves — forces stall
@@ -62,7 +63,16 @@ describe("provider.wrapSSE — SSEStallError integration", () => {
6263
const wrapped = wrapSSE(res, 2, ctl)
6364
const reader = wrapped.body!.getReader()
6465

65-
await expect(reader.read()).rejects.toBeInstanceOf(SSEStallError)
66+
const caught = await reader.read().then(
67+
() => undefined,
68+
(e) => e,
69+
)
70+
expect(MessageV2.SSEStallError.isInstance(caught)).toBe(true)
71+
if (!MessageV2.SSEStallError.isInstance(caught)) throw new Error("expected SSEStallError")
72+
expect(caught.data.message).toBe("SSE read timed out after 2ms")
73+
// signal.reason and the cancel reason must share identity with the thrown error
74+
expect(MessageV2.SSEStallError.isInstance(ctl.signal.reason)).toBe(true)
75+
expect(ctl.signal.reason).toBe(caught)
6676
})
6777

6878
test("does not wrap non-SSE responses", () => {

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { describe, expect, test } from "bun:test"
22
import { APICallError } from "ai"
3-
import { SSEStallError } from "../../src/provider/provider"
43
import { ProviderID } from "../../src/provider/schema"
54
import { MessageV2 } from "../../src/session/message-v2"
65

@@ -40,7 +39,7 @@ describe("session.message-v2.fromError — SSEStallError", () => {
4039
})
4140

4241
test("detects SSE stall through two-deep cause chain", () => {
43-
const stall = new SSEStallError("SSE read timed out")
42+
const stall = new MessageV2.SSEStallError({ message: "SSE read timed out" })
4443
const middle = new Error("middle")
4544
middle.cause = stall
4645
const outer = new Error("outer")
@@ -52,7 +51,7 @@ describe("session.message-v2.fromError — SSEStallError", () => {
5251
})
5352

5453
test("detects SSE stall when APICallError wraps SSEStallError", () => {
55-
const stall = new SSEStallError("SSE read timed out")
54+
const stall = new MessageV2.SSEStallError({ message: "SSE read timed out" })
5655
const apiError = new APICallError({
5756
message: "stream error",
5857
url: "https://api.githubcopilot.com/chat/completions",

packages/opencode/test/session/retry.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { setTimeout as sleep } from "node:timers/promises"
55
import { Effect, Exit, Layer, Pull, Schedule } from "effect"
66
import { SessionRetry } from "../../src/session/retry"
77
import { MessageV2 } from "../../src/session/message-v2"
8-
import { SSEStallError } from "../../src/provider/provider"
98
import { ProviderID } from "../../src/provider/schema"
109
import { AppRuntime } from "../../src/effect/app-runtime"
1110
import { SessionID } from "../../src/session/schema"
@@ -236,7 +235,7 @@ describe("session.retry.retryable", () => {
236235

237236
describe("SessionRetry.retryable — SSE stall round-trip", () => {
238237
test("retries SSEStallError after MessageV2.fromError round-trip", () => {
239-
const err = new SSEStallError("SSE read timed out")
238+
const err = new MessageV2.SSEStallError({ message: "SSE read timed out" })
240239
const obj = MessageV2.fromError(err, { providerID })
241240
expect(MessageV2.SSEStallError.isInstance(obj)).toBe(true)
242241
expect(SessionRetry.retryable(obj)).toBe("SSE read timed out")

0 commit comments

Comments
 (0)