Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions packages/opencode/src/provider/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,13 @@ import { withStatics } from "@/util/schema"

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

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

export class SSEStallError extends Error {
readonly _tag = "SSEStallError"
constructor(message: string) {
super(message)
this.name = "SSEStallError"
}
}

function shouldUseCopilotResponsesApi(modelID: string): boolean {
const match = /^gpt-(\d+)/.exec(modelID)
if (!match) return false
Expand Down Expand Up @@ -77,7 +73,7 @@ export function wrapSSE(res: Response, ms: number, ctl: AbortController) {
async pull(ctrl) {
const part = await new Promise<Awaited<ReturnType<typeof reader.read>>>((resolve, reject) => {
const id = setTimeout(() => {
const err = new SSEStallError(`SSE read timed out after ${ms}ms`)
const err = new MessageV2.SSEStallError({ message: `SSE read timed out after ${ms}ms` })
ctl.abort(err)
void reader.cancel(err)
reject(err)
Expand Down
31 changes: 29 additions & 2 deletions packages/opencode/src/session/message-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import { Snapshot } from "@/snapshot"
import { SyncEvent } from "../sync"
import { Database, NotFoundError, and, desc, eq, inArray, lt, or } from "@/storage"
import { MessageTable, PartTable, SessionTable } from "./session.sql"
import { ProviderError } from "@/provider"
// Import directly from provider/error (not the @/provider barrel) to avoid a
// circular import: provider/provider.ts now imports MessageV2 from this file,
// and the barrel re-exports provider.ts.
import * as ProviderError from "@/provider/error"
import { iife } from "@/util/iife"
import { errorMessage } from "@/util/error"
import { isMedia } from "@/util/media"
Expand Down Expand Up @@ -1092,6 +1095,30 @@ function hasSSEStallCause(e: unknown, depth = 0): boolean {
return false
}

// Extract the user-meaningful "SSE read timed out after Nms" string from an
// error that hasSSEStallCause matched. NamedSchemaError sets Error.prototype.message
// to the tag (`super(tag, options)` in named-schema-error.ts), so an in-process
// throw of MessageV2.SSEStallError caught and passed back through fromError must
// read `.data.message`, not `.message`, to recover the timing text. Plain Error
// instances (legacy or cross-realm) put it in `.message`. Walk the cause chain
// so nested wrappers still produce the original timing text.
function extractStallMessage(e: unknown, depth = 0): string {
if (depth > 8) return String(e)
if (!e || typeof e !== "object") return String(e)
const err = e as { data?: { message?: unknown }; message?: unknown; cause?: unknown }
if (err.data && typeof err.data.message === "string" && SSE_STALL_MESSAGE_RE.test(err.data.message)) {
return err.data.message
}
if (typeof err.message === "string" && SSE_STALL_MESSAGE_RE.test(err.message)) return err.message
if (err.cause) return extractStallMessage(err.cause, depth + 1)
// hasSSEStallCause already matched; produce best-effort text even if no node
// has the canonical "after Nms" timing format (e.g., test fixture passes
// "SSE read timed out" without the suffix).
if (err.data && typeof err.data.message === "string") return err.data.message
if (typeof err.message === "string") return err.message
return String(e)
}

export function fromError(
e: unknown,
ctx: { providerID: ProviderID; aborted?: boolean },
Expand Down Expand Up @@ -1144,7 +1171,7 @@ export function fromError(
).toObject()
case hasSSEStallCause(e):
return new SSEStallError(
{ message: e instanceof Error ? e.message : String(e) },
{ message: extractStallMessage(e) },
{ cause: e instanceof Error ? e : undefined },
).toObject()
Comment on lines 1172 to 1176
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractStallMessage() is meant to preserve the real timeout text when fromError() receives an in-process MessageV2.SSEStallError (whose .message is the tag). The current tests cover the fallback path (data.message without the after Nms suffix) but don’t assert the main regression being fixed: when data.message does match the canonical ^SSE read timed out after \d+ms$ format, fromError() should still emit that exact string (not "SSEStallError"). Consider adding a test that passes new MessageV2.SSEStallError({ message: "SSE read timed out after 2ms" }) into fromError() and asserts result.data.message === "SSE read timed out after 2ms".

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point — the canonical happy path was implicit in the chunk-timeout integration test but not asserted directly at the fromError boundary. Added a focused test in 3c31b2b that constructs a top-level MessageV2.SSEStallError with the canonical wrapSSE message and asserts result.data.message preserves it verbatim. Fails loudly if extractStallMessage ever regresses to copying .message (which would emit the literal tag 'SSEStallError'). 10/10 pass in message-v2-sse-stall.test.ts.

case APICallError.isInstance(e):
Expand Down
16 changes: 13 additions & 3 deletions packages/opencode/test/provider/chunk-timeout.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test"
import { resolveChunkTimeout, SSEStallError, wrapSSE } from "../../src/provider/provider"
import { resolveChunkTimeout, wrapSSE } from "../../src/provider/provider"
import { MessageV2 } from "../../src/session/message-v2"

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

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

await expect(reader.read()).rejects.toBeInstanceOf(SSEStallError)
const caught = await reader.read().then(
() => undefined,
(e) => e,
)
expect(MessageV2.SSEStallError.isInstance(caught)).toBe(true)
if (!MessageV2.SSEStallError.isInstance(caught)) throw new Error("expected SSEStallError")
expect(caught.data.message).toBe("SSE read timed out after 2ms")
// signal.reason and the cancel reason must share identity with the thrown error
expect(MessageV2.SSEStallError.isInstance(ctl.signal.reason)).toBe(true)
expect(ctl.signal.reason).toBe(caught)
})

test("does not wrap non-SSE responses", () => {
Expand Down
20 changes: 17 additions & 3 deletions packages/opencode/test/session/message-v2-sse-stall.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { describe, expect, test } from "bun:test"
import { APICallError } from "ai"
import { SSEStallError } from "../../src/provider/provider"
import { ProviderID } from "../../src/provider/schema"
import { MessageV2 } from "../../src/session/message-v2"

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

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

test("detects SSE stall when APICallError wraps SSEStallError", () => {
const stall = new SSEStallError("SSE read timed out")
const stall = new MessageV2.SSEStallError({ message: "SSE read timed out" })
const apiError = new APICallError({
message: "stream error",
url: "https://api.githubcopilot.com/chat/completions",
Expand All @@ -66,6 +65,21 @@ describe("session.message-v2.fromError — SSEStallError", () => {
expect(MessageV2.APIError.isInstance(result)).toBe(false)
})

test("preserves canonical wrapSSE message when fromError receives a top-level MessageV2.SSEStallError", () => {
// Regression for the F9 unification: schema-error instances have
// `.message === "SSEStallError"` (the tag, set by `super(tag, options)` in
// namedSchemaError), so fromError must read `.data.message` to recover the
// real timing text. If extractStallMessage ever regresses, the result here
// will be the literal string "SSEStallError" instead of the timing text.
const stall = new MessageV2.SSEStallError({ message: "SSE read timed out after 2ms" })

const result = MessageV2.fromError(stall, { providerID })

expect(MessageV2.SSEStallError.isInstance(result)).toBe(true)
if (!MessageV2.SSEStallError.isInstance(result)) throw new Error("Expected SSEStallError")
expect(result.data.message).toBe("SSE read timed out after 2ms")
})

test("hasSSEStallCause: tag-based detection still works", () => {
const tagged = Object.assign(new Error("anything"), { _tag: "SSEStallError" })
const result = MessageV2.fromError(tagged, { providerID })
Expand Down
3 changes: 1 addition & 2 deletions packages/opencode/test/session/retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { setTimeout as sleep } from "node:timers/promises"
import { Effect, Exit, Layer, Pull, Schedule } from "effect"
import { SessionRetry } from "../../src/session/retry"
import { MessageV2 } from "../../src/session/message-v2"
import { SSEStallError } from "../../src/provider/provider"
import { ProviderID } from "../../src/provider/schema"
import { AppRuntime } from "../../src/effect/app-runtime"
import { SessionID } from "../../src/session/schema"
Expand Down Expand Up @@ -236,7 +235,7 @@ describe("session.retry.retryable", () => {

describe("SessionRetry.retryable — SSE stall round-trip", () => {
test("retries SSEStallError after MessageV2.fromError round-trip", () => {
const err = new SSEStallError("SSE read timed out")
const err = new MessageV2.SSEStallError({ message: "SSE read timed out" })
const obj = MessageV2.fromError(err, { providerID })
expect(MessageV2.SSEStallError.isInstance(obj)).toBe(true)
expect(SessionRetry.retryable(obj)).toBe("SSE read timed out")
Expand Down
Loading