Skip to content

feat: add configurable upstream AI request timeouts - #1547

Merged
steebchen merged 2 commits into
mainfrom
steebchen/ai-api-timeout
Jan 29, 2026
Merged

steebchen merged 2 commits into
mainfrom
steebchen/ai-api-timeout

Conversation

@steebchen

@steebchen steebchen commented Jan 29, 2026 •

Copy link
Copy Markdown
Member

Summary

Implements a timeout mechanism for upstream AI API calls to ensure timeouts are caught gracefully before the overall gateway request times out.

  • AI_REQUEST_TIMEOUT_MS (default 4 minutes) is configurable via environment variables
  • Shorter than GATEWAY_TIMEOUT_MS to allow error handling before client timeout
  • Both streaming and non-streaming requests properly handle timeout errors (502 status)
  • Comprehensive test coverage with 3 new timeout-specific tests
  • Mock server supports timeout simulation for testing

Test plan

  • Run pnpm test:unit to verify all 369 unit tests pass including 3 new timeout tests
  • Existing tests continue to pass (20 tests in api.spec.ts)
  • Timeout tests simulate 5s upstream delay with 2s configured timeout

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added configurable timeout settings: gateway timeout and AI-request timeout environment variables with sensible defaults.
  • Improvements

    • Unified timeout/cancellation handling with clearer classification and tailored client notifications.
    • Enhanced logging to include timeout, cancellation, and upstream error context.
  • Tests

    • Added timeout-focused tests covering streaming, non‑streaming, and success-under-timeout scenarios; introduced test helpers to simulate upstream delays.
  • Documentation

    • Extended environment example with guidance for the new timeout variables.

✏️ Tip: You can customize this high-level summary in your review settings.

- Add timeout-config module with dynamic timeout getters
- Set AI_REQUEST_TIMEOUT_MS shorter than GATEWAY_TIMEOUT_MS to catch upstream timeouts
- Handle timeout errors (502) for both streaming and non-streaming requests
- Add mock server timeout simulation (TRIGGER_TIMEOUT keyword)
- Add comprehensive timeout handling tests with 2s timeout
- Properly log timeout errors with TimeoutError status text

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jan 29, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

Introduces centralized timeout configuration, integrates unified timeout/cancellation signals and upstream-timeout handling into the chat flow, adds tests for timeout scenarios, and extends the mock OpenAI server with message-triggered artificial delays. No behavioral changes outside timeout/cancellation/error paths.

Changes

Cohort / File(s) Summary
Env & Timeout Config
.env.example, apps/gateway/src/lib/timeout-config.ts
Documented GATEWAY_TIMEOUT_MS and AI_REQUEST_TIMEOUT_MS; added a new module providing getters, legacy constants, timeout signal creation, combined abort signal creation, and timeout/cancellation detection helpers.
Chat handler updates
apps/gateway/src/chat/chat.ts
Replaced per-request cancellation with createCombinedSignal, added isTimeoutError detection, distinct upstream-timeout error path with logging, SSE error emission, and extended cancellation/error logging; unified fetch signal usage across streaming and non-streaming flows.
Tests & Mocks
apps/gateway/src/api.spec.ts, apps/gateway/src/test-utils/mock-openai-server.ts, apps/gateway/src/test-utils/...
Added timeout handling test suite (non-streaming, streaming, success-under-timeout), imported new readAll helper in tests, and added extractTimeoutDelay/delay utilities in mock server to simulate upstream delays; note: tests were duplicated in the spec file in the diff.
Test helpers export change
apps/gateway/src/test-utils/test-helpers.js
Export readAll (imported by tests).

Sequence Diagram

sequenceDiagram
    participant Client
    participant Gateway as Gateway<br/>(apps/gateway/src/chat/chat.ts)
    participant TimeoutCfg as Timeout Config<br/>(apps/gateway/src/lib/timeout-config.ts)
    participant Provider as Upstream Provider
    participant Logger as Logger / SSE

    Client->>Gateway: Send chat request
    Gateway->>TimeoutCfg: createCombinedSignal(cancellationController)
    TimeoutCfg->>TimeoutCfg: AbortSignal.timeout(AI_REQUEST_TIMEOUT_MS)
    TimeoutCfg-->>Gateway: fetchSignal
    Gateway->>Provider: fetch(..., { signal: fetchSignal })

    alt Upstream timeout fires
        TimeoutCfg->>Provider: aborts via signal
        Provider-->>Gateway: throws TimeoutError
        Gateway->>Logger: log upstream_timeout with context
        Gateway->>Logger: emit SSE error (upstream_timeout)
        Gateway-->>Client: terminate stream / send error event
    else Request cancelled
        Gateway->>Logger: log cancellation & billing notes
        Gateway->>Logger: emit SSE cancellation event
        Gateway-->>Client: terminate stream (cancellation)
    else Upstream responds in time
        Provider-->>Gateway: response
        Gateway-->>Client: deliver content (stream or final response)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

auto-merge

Suggested reviewers

  • smakosh
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add configurable upstream AI request timeouts' accurately and concisely summarizes the main change—implementing environment-configurable timeout handling for upstream AI API calls.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@steebchen
steebchen enabled auto-merge January 29, 2026 20:07

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2796-2889: Timeout error handler in the streaming block (the
branch that calls isTimeoutError(error)) sends an "error" SSE but does not send
the final "[DONE]" event; update that handler (the code that calls
stream.writeSSE with event: "error", id: String(eventId++)) to also call
stream.writeSSE a second time sending the completion message/data "[DONE]" (same
format used in other handlers) after the error event so clients receive the
terminal event and the stream is closed consistently.
🧹 Nitpick comments (1)
apps/gateway/src/lib/timeout-config.ts (1)

32-35: Consider removing or deprecating these legacy exports.

These constants are read at module load time, which means they won't reflect runtime environment changes (e.g., in tests). The comment warns against using them, but they could lead to subtle bugs if used inadvertently.

Comment thread apps/gateway/src/chat/chat.ts
@steebchen
steebchen added this pull request to the merge queue Jan 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 29, 2026
@steebchen
steebchen added this pull request to the merge queue Jan 29, 2026
Merged via the queue into main with commit cd89e0c Jan 29, 2026
13 of 14 checks passed
@steebchen
steebchen deleted the steebchen/ai-api-timeout branch January 29, 2026 21:29
steebchen added a commit that referenced this pull request Jan 29, 2026
Implements a timeout mechanism for upstream AI API calls to ensure
timeouts are caught gracefully before the overall gateway request times
out.

- AI_REQUEST_TIMEOUT_MS (default 4 minutes) is configurable via
environment variables
- Shorter than GATEWAY_TIMEOUT_MS to allow error handling before client
timeout
- Both streaming and non-streaming requests properly handle timeout
errors (502 status)
- Comprehensive test coverage with 3 new timeout-specific tests
- Mock server supports timeout simulation for testing

- Run `pnpm test:unit` to verify all 369 unit tests pass including 3 new
timeout tests
- Existing tests continue to pass (20 tests in api.spec.ts)
- Timeout tests simulate 5s upstream delay with 2s configured timeout

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

* **New Features**
* Added configurable timeout settings: gateway timeout and AI-request
timeout environment variables with sensible defaults.

* **Improvements**
* Unified timeout/cancellation handling with clearer classification and
tailored client notifications.
* Enhanced logging to include timeout, cancellation, and upstream error
context.

* **Tests**
* Added timeout-focused tests covering streaming, non‑streaming, and
success-under-timeout scenarios; introduced test helpers to simulate
upstream delays.

* **Documentation**
* Extended environment example with guidance for the new timeout
variables.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
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