Skip to content

Add HTTP_POST_TIMEOUT to TimeoutMiddleware config (#5941)#6150

Merged
beastoin merged 4 commits intomainfrom
fix/post-timeout-5941
Mar 30, 2026
Merged

Add HTTP_POST_TIMEOUT to TimeoutMiddleware config (#5941)#6150
beastoin merged 4 commits intomainfrom
fix/post-timeout-5941

Conversation

@beastoin
Copy link
Copy Markdown
Collaborator

@beastoin beastoin commented Mar 29, 2026

Summary

Adds HTTP_POST_TIMEOUT to the methods_timeout dict in backend/main.py, enabling per-method timeout control for POST requests.

Fixes #5941sync-local-files 504 timeouts on large payloads. POST was the only HTTP method missing from the timeout config (omitted since the original ac328e5b commit in Jan 2025).

Changes

  • backend/main.py: Add "POST": os.environ.get('HTTP_POST_TIMEOUT') to methods_timeout
  • backend/tests/unit/test_timeout_middleware.py: 3 new tests covering POST timeout override, None fallback with {"POST": None}, and AST verification of both key and env var name

Deployment

After merge, set on backend-sync Cloud Run:

HTTP_POST_TIMEOUT=600

This matches backend (main) which already has HTTP_DEFAULT_TIMEOUT=600 — POST there already gets 600s via fallback. The fix is specifically for backend-sync (and any other service) where the default is 120s.

Current env var state (from mon's audit):

Service HTTP_DEFAULT_TIMEOUT HTTP_POST_TIMEOUT (after this PR)
backend (main) 600s set to 600s
backend-sync 120s (hardcoded) set to 600sfixes #5941
All others 120s (hardcoded) unset (120s fallback)

WebSocket impact: None

HTTP_POST_TIMEOUT does NOT affect WebSocket connections (/v4/listen):

  1. WebSocket upgrade requests are GET, not POST
  2. Starlette BaseHTTPMiddleware only handles HTTP requests — WebSocket connections go through a different ASGI path
  3. NO_SOCKET_TIMEOUT in helm values is for a separate layer

Risks

  • Scope: HTTP_POST_TIMEOUT applies to ALL POST routes, not just /v1/sync-local-files. Consistent with GET/PUT/PATCH/DELETE.
  • No behavior change without env var: None is filtered by _parse_methods_timeout → POST falls back to HTTP_DEFAULT_TIMEOUT.

Tests

16/16 pass (pytest backend/tests/unit/test_timeout_middleware.py):

  • 13 existing tests unchanged
  • test_post_timeout_override: POST with explicit timeout → 504 on slow handler
  • test_post_timeout_none_falls_back_to_default: {"POST": None} → skipped → default timeout
  • test_main_methods_timeout_includes_post: AST-parses main.py, asserts POST key + HTTP_POST_TIMEOUT env var

Live Test Evidence

L1 (standalone backend)

Parsed methods_timeout: {'POST': 2.0}
Default timeout: 120.0
POST resolved: 2.0
Without env var, parsed: {}
POST falls back to default: 120.0

Fast POST: 200 (expect 200) ✓
Slow POST: 504 (expect 504) ✓

L2 (backend + client integrated)

GET /health: 200 {'status': 'ok'} ✓
POST /upload-fast: 200 {'received': 1500} ✓
POST /upload-slow: 504 ✓

Changed-path coverage

Path ID Changed path Happy-path Non-happy-path L1 L2
P1 main.py:121 POST in methods_timeout POST completes within timeout → 200 POST exceeds timeout → 504 PASS PASS
P2 test_timeout_middleware.py tests 16/16 pass N/A (test code) PASS N/A

by AI for @beastoin

POST was omitted from methods_timeout when TimeoutMiddleware was first
added (ac328e5). This meant POST endpoints had no env var override
for the 120s default, causing 504s on /v1/sync-local-files for large
payloads. Adding the env var entry enables operators to tune it.

Closes #5941

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 29, 2026

Greptile Summary

This PR adds a single missing entry — "POST": os.environ.get('HTTP_POST_TIMEOUT') — to the methods_timeout dict in backend/main.py, making POST consistent with the other HTTP methods (GET, PUT, PATCH, DELETE) that already had configurable timeout overrides via environment variables.

  • The fix is correct: _parse_methods_timeout in utils/other/timeout.py explicitly skips None values, so leaving HTTP_POST_TIMEOUT unset preserves the existing 120s default with no behavior change.
  • The implementation is fully consistent with the existing pattern for other methods.
  • The middleware's dispatch method falls back to self.default_timeout for any method not in self.methods_timeout, so the gap was a configuration affordance bug rather than a functional one — operators simply couldn't override the POST timeout.
  • No tests, documentation, or other files require changes for this one-liner fix.

Confidence Score: 5/5

Safe to merge — minimal one-line change, no behavior change until HTTP_POST_TIMEOUT is explicitly set, and the None-filtering logic in _parse_methods_timeout is verified correct.

The change is a trivial one-liner that fills an obvious omission in the existing pattern. The downstream _parse_methods_timeout logic was already built to handle None (unset env var) gracefully. No logic errors, no security concerns, no API contract changes. All remaining feedback would be P2 or lower.

No files require special attention.

Important Files Changed

Filename Overview
backend/main.py Adds "POST": os.environ.get('HTTP_POST_TIMEOUT') to the methods_timeout dict, filling the gap that prevented operators from overriding POST request timeouts via env var. The None fallback path through _parse_methods_timeout is correct and no behavior changes until the env var is explicitly set.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming HTTP Request] --> B{Has x-request-start-time header?}
    B -- Yes --> C{Request age > max_age + clock_skew?}
    C -- Yes --> D[Return 408 clock_skew]
    C -- No --> E[Lookup timeout for method]
    B -- No --> E
    E --> F{method in methods_timeout?}
    F -- Yes --> G[Use per-method timeout\ne.g. HTTP_POST_TIMEOUT]
    F -- No --> H[Use default_timeout\nHTTP_DEFAULT_TIMEOUT = 120s]
    G --> I[asyncio.wait_for call_next]
    H --> I
    I -- success --> J[Return response]
    I -- TimeoutError --> K[Return 504 Gateway Timeout]
Loading

Reviews (1): Last reviewed commit: "Add HTTP_POST_TIMEOUT to TimeoutMiddlewa..." | Re-trigger Greptile

beastoin and others added 3 commits March 30, 2026 01:01
- test_post_timeout_override: POST returns 504 when exceeding POST-specific timeout
- test_post_timeout_none_falls_back_to_default: POST falls back to HTTP_DEFAULT_TIMEOUT when unset
- test_main_methods_timeout_includes_post: methods_timeout dict accepts POST key

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace test_main_methods_timeout_includes_post with an AST-based
check that directly verifies backend/main.py contains POST in the
methods_timeout dict, avoiding false-positive from constructing a
separate dict manually.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- test_post_timeout_none_falls_back_to_default now passes {"POST": None}
  to match what main.py actually builds when env var is unset
- test_main_methods_timeout_includes_post now verifies the value is wired
  to HTTP_POST_TIMEOUT env var, not just that the POST key exists

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@beastoin
Copy link
Copy Markdown
Collaborator Author

Test Evidence (#5941)

Unit Tests — 16/16 pass

pytest backend/tests/unit/test_timeout_middleware.py -v
...
16 passed in 30.36s

3 new tests added:

  • test_post_timeout_override: POST with explicit timeout → 504 on slow handler
  • test_post_timeout_none_falls_back_to_default: {"POST": None} (matching main.py when env unset) → skipped by _parse_methods_timeout → default timeout applies
  • test_main_methods_timeout_includes_post: AST-parses main.py to assert POST key exists AND value is wired to HTTP_POST_TIMEOUT env var

L1 — Standalone backend

Parsed methods_timeout: {'POST': 2.0}
Default timeout: 120.0
POST resolved: 2.0
Without env var, parsed: {}
POST falls back to default: 120.0

Fast POST: 200 (expect 200) ✓
Slow POST: 504 (expect 504) ✓

L2 — Backend + client integrated

GET /health: 200 {'status': 'ok'} ✓
POST /upload-fast: 200 {'received': 1500} ✓
POST /upload-slow: 504 ✓

Changed-path coverage

Path ID Changed path Happy-path Non-happy-path L1 L2
P1 main.py:121 POST in methods_timeout POST completes → 200 POST exceeds timeout → 504 PASS PASS
P2 test_timeout_middleware.py tests 16/16 pass N/A (test code) PASS N/A

WebSocket impact: None

HTTP_POST_TIMEOUT does NOT affect /v4/listen — WS upgrade is GET, and BaseHTTPMiddleware only handles HTTP, not WebSocket.

by AI for @beastoin

@beastoin
Copy link
Copy Markdown
Collaborator Author

lgtm

@beastoin beastoin merged commit 177bb36 into main Mar 30, 2026
2 checks passed
@beastoin beastoin deleted the fix/post-timeout-5941 branch March 30, 2026 02:38
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.

sync-local-files 504 timeouts on large payloads (>120s pipeline)

1 participant