Skip to content

fix(transport): propagate SDK errors instead of silent 200-empty#36

Merged
lesquel merged 1 commit into
mainfrom
fix/sdk-error-consistency
May 17, 2026
Merged

fix(transport): propagate SDK errors instead of silent 200-empty#36
lesquel merged 1 commit into
mainfrom
fix/sdk-error-consistency

Conversation

@lesquel

@lesquel lesquel commented May 17, 2026

Copy link
Copy Markdown
Owner

Summary

Resolves the SDK result.error consistency item of #33 (Tier-2; convergent across the domain-logic + silent-failures passes).

Six SDK-backed read handlers did json(result.data ?? [], 200) without checking result.error. On SDK failure (IPC down, session vanished, OpenCode restarting) the dashboard rendered "empty" instead of surfacing the error — silent failure, inconsistent with getSession/updateSession/deleteSession/postSessionPrompt which already check.

Fix — mirrors the existing deleteSession precedent exactly

Handler File Behavior on SDK error
listSessions sessions.ts either of the 2 calls errors → fail (no half-empty 200)
getSessionMessages / getSessionDiff / getSessionChildren sessions.ts not-found → 404; generic → 500 + logger.error
listTools sdk-proxy.ts generic → 500 + logger.error
getStatus system.ts either of the 2 calls errors → 500 + logger.error
  • Success/empty paths byte-identical — same ?? []/?? {} fallback, status 200, CORS headers. Only error early-returns were added.
  • Status codes/error codes match the in-repo deleteSession/postSessionPrompt precedent (404 NOT_FOUND / 500 SDK_ERROR) — no new convention.
  • Extracted sdkErrorMessage() from deleteSession's inline logic (behaviorally identical) to avoid 4× duplication.
  • getSession deliberately untouched (its 404-on-any-error is separately tracked on feat: v1.21.0 audit follow-ups — broken lint script + architecture/quality debt (non-security) #25).

Testing

  • +23 tests (error → non-200 + logger.error called; not-found → 404; success/empty → original 200 shape unchanged). Independent spy logger per test, non-flaky.
  • bun test 647/0 · bunx tsc --noEmit clean
  • Independent fresh-context review: APPROVE, no blocking issues — all 7 checks PASS, incl. an exhaustive old-vs-new diff confirming the success path is byte-identical. The single nit (unused test import) is fixed.

Merge note

Independent off main. Touches system.ts (getStatus) — open PR #34 also touches system.ts but a different function (rotateAuthToken); non-overlapping hunks → clean 3-way merge in any order. sessions.ts/sdk-proxy.ts don't overlap #34/#35.

Refs #33 (Tier-2, partial — does not close).

Second-wave audit (#33 Tier-2), convergent across two review passes.

Six SDK-backed read handlers returned `json(result.data ?? [], 200)`
without checking `result.error`. When the OpenCode SDK returned
{ data: undefined, error } (IPC down, session vanished, OpenCode
restarting) the dashboard saw "empty" instead of an error — silent
failure, and inconsistent with getSession/updateSession/deleteSession/
postSessionPrompt which already check.

Fixed by mirroring the existing deleteSession precedent exactly:
- listSessions, getSessionMessages, getSessionDiff, getSessionChildren
  (sessions.ts), listTools (sdk-proxy.ts), getStatus (system.ts) now
  inspect result.error: not-found-style → 404 NOT_FOUND; generic →
  deps.logger.error + 500 SDK_ERROR. Success/empty paths are
  byte-identical (same `?? []`/`?? {}` fallback, same 200, same CORS).
- listSessions/getStatus make two SDK calls — either error now fails
  cleanly (no half-empty 200).
- Extracted sdkErrorMessage() helper from deleteSession's inline logic
  (behaviorally identical) to avoid duplication.
- getSession deliberately untouched (separately tracked on #25).

+23 tests. bun test 647/0, tsc clean. Independent fresh-context review:
APPROVE, no blocking issues (all 7 checks PASS incl. success-path
byte-identical diff). The one nit (unused test import) is fixed here.

Refs #33
@lesquel lesquel merged commit 3719947 into main May 17, 2026
2 checks passed
@lesquel lesquel deleted the fix/sdk-error-consistency branch May 17, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant