Skip to content

fix(api): remove ReadTimeout to prevent premature body-read failures under high concurrency (rebase of #3416) - #3722

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/api-remove-read-timeout-v2
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/api-remove-read-timeout-v2

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Rebased follow-up to #3416, which @ryan-sana closed with an invitation to "rebase the one-line change onto current main." The ReadTimeout code is still present on main (packages/api/main.go:254), so the problem in #3415 is unaddressed.

Summary

  • Remove ReadTimeout: maxReadTimeout (10s) from the http.Server.
  • Remove the now-unused maxReadTimeout constant.
  • Keep ReadHeaderTimeout: 5s (slowloris protection) and WriteTimeout: 75s unchanged.

Net diff: 2 deletions.

Why

ReadTimeout is measured from TCP accept, so it includes the time the serve goroutine spends queued in the Go scheduler before it runs. Under high concurrency that queue time alone can exceed 10s, so the deadline fires before io.ReadAll ever executes — surfacing as i/o timeout body-read errors that look like client disconnects or auth failures.

The intended per-request ceiling is the RequestTimeout middleware (cfg.RequestTimeout, a context deadline — still wired at main.go:173), which ReadTimeout preempts and so is never reached in practice.

Once headers have arrived (guarded by ReadHeaderTimeout), there's no meaningful slow-body attack surface: request bodies on this API are small JSON payloads sent in a single TCP segment. WriteTimeout (asserted > cfg.RequestTimeout at compile time) still bounds the response side.

Verification on current main

  • ReadTimeout/maxReadTimeout confirmed still present and used only at the server construction site (no other references).
  • RequestTimeout middleware, ReadHeaderTimeout, and the WriteTimeout compile-time assertion all still in place — so the diagnosis holds unchanged.
  • go build ./packages/api/... and go vet pass.

Test plan

  • Staging: replay ≥100 simultaneous sandbox creates; confirm i/o timeout body-read errors disappear.
  • /health still responds under load.
  • Slowloris still rejected within 5s (ReadHeaderTimeout): a client that opens a connection but never sends headers is dropped.

Closes #3415. Supersedes #3416.

…under high concurrency

ReadTimeout (10s) is measured from TCP accept, so it includes time the serve
goroutine spends queued in the Go scheduler before it runs. Under high
concurrency that queue time alone can exceed 10s, so the deadline fires before
io.ReadAll ever executes - surfacing as i/o timeout body-read errors that look
like client disconnects or auth failures.

The intended per-request ceiling is the RequestTimeout middleware
(cfg.RequestTimeout, the context deadline), which ReadTimeout preempts and so
is never reached. Remove ReadTimeout and the now-unused maxReadTimeout const.

ReadHeaderTimeout (5s) is kept for slowloris protection: once headers arrive
there is no meaningful slow-body surface, since request bodies on this API are
small JSON payloads sent in a single TCP segment. WriteTimeout (75s, asserted
> cfg.RequestTimeout at compile time) is unchanged.

Rebased onto current main at the maintainers invitation on e2b-dev#3416. Closes e2b-dev#3415.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

api: ReadTimeout: 10s kills body reads under high concurrency, conflicting with requestTimeout: 70s

2 participants