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.

Dshamir added a commit to Dshamir/AI-Legal that referenced this pull request May 24, 2026
- Add RLS to all public tables with deny-all default policy and
  auto-enable event trigger for future tables. PostgREST anon role
  can no longer read any data. Prisma service-role bypasses RLS (PR willchen96#145)
- Wrap runLLMStream() in Promise.race with 180s configurable timeout;
  sends SSE error event on timeout and closes connection (PR willchen96#112)
- Cap download-zip document_ids array at 50 to prevent memory
  exhaustion from unbounded batch downloads (PR willchen96#111)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dshamir added a commit to Dshamir/AI-Legal that referenced this pull request May 24, 2026
…tegration

- CHANGELOG: add security hardening and feature entries for PRs willchen96#158,
  willchen96#81, willchen96#76, willchen96#79, willchen96#145, willchen96#112, willchen96#111, willchen96#110, willchen96#155, willchen96#157, willchen96#59
- ROADMAP: mark 12 new items as completed
- CLAUDE.md: add sanitize.ts, streamTimeout.ts, credits.ts to lib index,
  update test count to 40
- README: update API endpoints table (chat pagination, workflow export),
  security row (HKDF, RLS, prompt defense), encryption row

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
amal66 added a commit to amal66/mike that referenced this pull request May 25, 2026
Chapter: 13 - Bounded external calls.

Plain-English map:
Add a three-minute timeout around LLM server-sent-event streams so a stalled
provider call cannot hold a connection open forever.

Why it matters:
External services can hang. Without a timeout, one stuck call can consume
server resources until something outside the app kills it.

Principle:
Every external dependency call should have a clear time boundary.

Precedent borrowed:
Upstream PR willchen96#112.

Upstream base: willchen96/mike@d39f580.
Original local commit: 8992d98.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment