Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway: add configurable response write timeout #818

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gammazero
Copy link
Contributor

@gammazero gammazero commented Jan 28, 2025

Closes #679
Replacement for #812

Copy link

codecov bot commented Jan 28, 2025

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.40%. Comparing base (b0125d1) to head (0b98918).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
gateway/handler.go 77.77% 2 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #818      +/-   ##
==========================================
- Coverage   60.48%   60.40%   -0.08%     
==========================================
  Files         244      244              
  Lines       31100    31108       +8     
==========================================
- Hits        18811    18791      -20     
- Misses      10611    10634      +23     
- Partials     1678     1683       +5     
Files with missing lines Coverage Δ
gateway/gateway.go 83.54% <ø> (ø)
gateway/handler.go 76.25% <77.77%> (-0.02%) ⬇️

... and 13 files with indirect coverage changes

@gammazero gammazero marked this pull request as ready for review January 28, 2025 17:51
@gammazero gammazero requested review from lidel and a team as code owners January 28, 2025 17:51
@hsanjuan
Copy link
Contributor

hsanjuan commented Feb 3, 2025

This doesn't act as nginx's proxy_read_timeout though?... i.e. a roundtrip should not be killed just because it took 30 seconds... the problem is 30 seconds without writing any data on the response body.

Instead of #812, which goes in that direction, perhaps we can get away with it by setting ResponseHeaderTimeout in the client configuration. Normally if a request lasts more than 30 seconds without any data coming back, the reason is that we are still looking for the content, and I doubt we have written any response headers by then.

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.

gateway: ability to set response write timeout
2 participants