Skip to content

Disable the 2 minutes default incoming timeout by default, matches proxyTimeout defaut behavior #1273

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Johny101
Copy link

Hi

This modification will save head hakes to those playing with long HTTP request.

Thanks!

Johny

DESCRIPTION:
Now, if the incoming timeout option is not set or if set to 0, it will be disabled by setting the req.socket timeout to 0.
This replace the default node.js 2 minutes timeout and now support disabling it.

It will have the same default behavior than the incoming proxyTimeout option.

TEST:
In a Windows and Arm-Linux environment, set option.timeout to 0, to 5 minutes and no timeout option.
Run a 8 minutes long request. Confirm expected behavior for all cases.

DESCRIPTION:
If the incoming timeout option is not set or if set to 0, it will be disabled.
It will now have the same default behavior than the incoming proxyTimeout option.

This replace the default node.js 2 minutes timeout and now support disabling it.

TEST:
In a Windows and Arm-Linux environment, set option.timeout to 0, to 5 minutes and remove option.
Run a 8 minutes long request. Confirm expected behavior for all cases.
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@a3fe02d). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1273   +/-   ##
=========================================
  Coverage          ?   92.35%           
=========================================
  Files             ?        6           
  Lines             ?      314           
  Branches          ?        0           
=========================================
  Hits              ?      290           
  Misses            ?       24           
  Partials          ?        0
Impacted Files Coverage Δ
lib/http-proxy/passes/web-incoming.js 98.33% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3fe02d...86f8c9b. Read the comment docs.

@pfcoperez
Copy link

pfcoperez commented May 19, 2020

@indexzero Is anything preventing this PR from being merged. The issue it fix bites quite often and there seem not to have easy workarounds?

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.

4 participants