Skip to content

fix: 3-minute timeout on SSE LLM streams#112

Open
bmersereau wants to merge 4 commits into
willchen96:mainfrom
bmersereau:fix/100-sse-stream-timeout
Open

fix: 3-minute timeout on SSE LLM streams#112
bmersereau wants to merge 4 commits into
willchen96:mainfrom
bmersereau:fix/100-sse-stream-timeout

Conversation

@bmersereau
Copy link
Copy Markdown

@bmersereau bmersereau commented May 13, 2026

Summary

  • Wraps runLLMStream in both POST /chat and POST /projects/:projectId/chat with a Promise.race against a 180-second timeout
  • On timeout, writes { type: "error", message: "Request timed out" } SSE event followed by [DONE] and closes the connection
  • Prevents stalled upstream LLM API calls from holding SSE connections open indefinitely under load

Closes #100
Closes #114
Closes #117
Closes #119

Changes

  • backend/src/routes/chat.tsSTREAM_TIMEOUT_MS = 180_000, Promise.race wrapping runLLMStream, timeout-specific error message in catch
  • backend/src/routes/projectChat.ts — same pattern; STREAM_TIMEOUT_MS const placed after all imports
  • backend/src/lib/__tests__/sseTimeout.test.ts — static analysis tests verifying both routes use Promise.race and the timeout
  • backend/tsconfig.json — exclude __tests__ from tsc build
  • backend/vitest.config.ts — include filter scoping tests to src/ to exclude compiled dist/ artifacts
  • backend/package.json"test": "vitest run" script added

Test plan

  • Unit tests added and passing (3/3)
  • Backend build passes

Copy link
Copy Markdown
Author

@bmersereau bmersereau left a comment

Choose a reason for hiding this comment

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

PR Review

Summary

Wraps runLLMStream in both POST /chat and POST /projects/:projectId/chat with a 180-second Promise.race timeout. On timeout the handler writes a typed error SSE event and [DONE] before closing the connection. Three static-analysis tests verify both routes use Promise.race and the timeout constant.

Findings

  • [severity:praise] clearTimeout(timerId!) called on the happy path — prevents timer leak after successful stream
  • [severity:praise] STREAM_TIMEOUT_MS = 180_000 is placed correctly after all imports in projectChat.ts ✓ (issue #119 resolved)
  • [severity:praise] vi.mock hoisting pattern not applicable here; STREAM_TIMEOUT_MS placement verified
  • [severity:nit] The timerId! non-null assertion is needed because TypeScript can't prove timerId is assigned in the Promise constructor. This is a known unavoidable pattern; no change needed

Specific checks

  • "test": "vitest run" in package.json ✓
  • vitest.config.ts include filter present ✓
  • STREAM_TIMEOUT_MS after all imports in projectChat.ts ✓ (issue #119)
  • Tests pass: 3/3 ✓

Verdict

Approve — clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment