http: fix incorrect headersTimeout measurement (alt) by OrKoN · Pull Request #32329 · nodejs/node · GitHub

For keep-alive connections, the headersTimeout may fire during
subsequent request because the measurement was reset after
a request and not before a request.

Fixes: #27363

Alternative implementation for: #30184

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. Fixes: nodejs#27363

for the response parsing by the client the headers timeout is not relevant

time tracking is moved to c++ code

should I check the return value? anything else needed to mark the parsing error?

@OrKoN Since you’d be return;ing here either way, whether the call threw an exception or not, I don’t think so. You can wrap the call in USE() to make it explicit that you’re discarding the return value because of that.

is there a good way to avoid having an extra callback here?

@OrKoN I don’t think so. But if it only fires in the timeout case, that should be okay.

There is one test case failing:

=== release test-graph.http ===
Path: async-hooks/test-graph.http
--- stderr ---
'timeout:1' expected to be triggered by 'tcp:2', but was triggered by 'httpincomingmessage:1' instead.
assert.js:102
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 0

    at verifyGraph (/home/runner/work/node/node/test/async-hooks/verify-graph.js:100:10)
    at process.<anonymous> (/home/runner/work/node/node/test/async-hooks/test-graph.http.js:29:3)
    at process.emit (events.js:327:22) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 1,
  expected: 0,
  operator: 'strictEqual'
}
Command: out/Release/node /home/runner/work/node/node/test/async-hooks/test-graph.http.js

I am not familiar with async hooks implementation and if anyone can suggest what's wrong here, I'd really appreciate.

I think the difference in the async-hooks test is caused by the call to nowDate which was setting a timer and which is now removed and the internal caching timer is triggered by another call (probably, utcDate).

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/543/

Benchmark CI looks ok to me

Details ``` confidence improvement accuracy (*) (**) (***) 23:20:26 http/bench-parser.js n=100000 len=16 * 2.15 % ±1.64% ±2.19% ±2.86% 23:20:26 http/bench-parser.js n=100000 len=32 1.51 % ±2.93% ±3.90% ±5.08% 23:20:26 http/bench-parser.js n=100000 len=4 0.95 % ±3.47% ±4.67% ±6.17% 23:20:26 http/bench-parser.js n=100000 len=8 5.72 % ±6.32% ±8.50% ±11.23% 23:20:26 http/check_invalid_header_char.js n=1000000 input='' -1.69 % ±4.17% ±5.55% ±7.23% 23:20:26 http/check_invalid_header_char.js n=1000000 input='1' -4.21 % ±4.76% ±6.37% ±8.37% 23:20:26 http/check_invalid_header_char.js n=1000000 input='20091' * -3.33 % ±3.01% ±4.00% ±5.21% 23:20:26 http/check_invalid_header_char.js n=1000000 input='close' ** -5.49 % ±3.15% ±4.21% ±5.51% 23:20:26 http/check_invalid_header_char.js n=1000000 input='en-US' -2.43 % ±4.33% ±5.77% ±7.51% 23:20:26 http/check_invalid_header_char.js n=1000000 input='foo\\nbar' 0.34 % ±4.23% ±5.63% ±7.33% 23:20:26 http/check_invalid_header_char.js n=1000000 input='group_acmeair' 1.03 % ±2.87% ±3.82% ±4.97% 23:20:26 http/check_invalid_header_char.js n=1000000 input='gzip' -2.76 % ±4.15% ±5.53% ±7.20% 23:20:26 http/check_invalid_header_char.js n=1000000 input='keep-alive' 0.12 % ±3.33% ±4.44% ±5.77% 23:20:26 http/check_invalid_header_char.js n=1000000 input='LONG_AND_INVALID' -1.62 % ±1.80% ±2.39% ±3.12% 23:20:26 http/check_invalid_header_char.js n=1000000 input='private' -3.55 % ±4.37% ±5.82% ±7.59% 23:20:26 http/check_invalid_header_char.js n=1000000 input='SAMEORIGIN' * -4.27 % ±3.66% ±4.89% ±6.39% 23:20:26 http/check_invalid_header_char.js n=1000000 input='Sat, 07 May 2016 16:54:48 GMT' -2.39 % ±2.54% ±3.38% ±4.40% 23:20:26 http/check_invalid_header_char.js n=1000000 input='text/html; charset=utf-8' 1.63 % ±3.17% ±4.24% ±5.57% 23:20:26 http/check_invalid_header_char.js n=1000000 input='text/plain' * -3.60 % ±3.36% ±4.48% ±5.86% 23:20:26 http/check_invalid_header_char.js n=1000000 input='\\t\\t\\t\\t\\t\\t\\t\\t\\t\\tFoo bar baz' * -4.19 % ±3.20% ±4.28% ±5.62% 23:20:26 http/check_invalid_header_char.js n=1000000 input='\\x7F' -1.18 % ±4.16% ±5.55% ±7.25% 23:20:26 http/check_invalid_header_char.js n=1000000 input='中文呢' 0.66 % ±4.67% ±6.22% ±8.10% 23:20:26 http/check_is_http_token.js n=1000000 key=':' -1.89 % ±6.70% ±8.92% ±11.62% 23:20:26 http/check_is_http_token.js n=1000000 key='((((())))' -2.34 % ±6.06% ±8.07% ±10.53% 23:20:26 http/check_is_http_token.js n=1000000 key='@@' -0.81 % ±6.30% ±8.39% ±10.92% 23:20:26 http/check_is_http_token.js n=1000000 key='Accept-Ranges' -0.88 % ±2.90% ±3.86% ±5.02% 23:20:26 http/check_is_http_token.js n=1000000 key=':alternate-protocol' 0.62 % ±7.26% ±9.66% ±12.58% 23:20:26 http/check_is_http_token.js n=1000000 key='alternate-protocol:' -3.34 % ±4.01% ±5.33% ±6.94% 23:20:26 http/check_is_http_token.js n=1000000 key='alternate-protocol' -2.35 % ±5.15% ±6.85% ±8.91% 23:20:26 http/check_is_http_token.js n=1000000 key='alt-svc' 0.73 % ±5.97% ±7.95% ±10.35% 23:20:26 http/check_is_http_token.js n=1000000 key='Cache-Control' 1.64 % ±3.24% ±4.32% ±5.64% 23:20:26 http/check_is_http_token.js n=1000000 key='Connection' -0.01 % ±2.94% ±3.91% ±5.09% 23:20:26 http/check_is_http_token.js n=1000000 key='Content-Encoding' -1.71 % ±4.07% ±5.42% ±7.07% 23:20:26 http/check_is_http_token.js n=1000000 key='content-length' -0.18 % ±2.90% ±3.86% ±5.03% 23:20:26 http/check_is_http_token.js n=1000000 key='Content-Location' -2.30 % ±4.64% ±6.17% ±8.03% 23:20:26 http/check_is_http_token.js n=1000000 key='content-type' 0.61 % ±3.50% ±4.66% ±6.07% 23:20:26 http/check_is_http_token.js n=1000000 key='Content-Type' 7.05 % ±8.46% ±11.31% ±14.82% 23:20:26 http/check_is_http_token.js n=1000000 key='date' -0.27 % ±2.92% ±3.88% ±5.05% 23:20:26 http/check_is_http_token.js n=1000000 key='ETag' 4.21 % ±5.85% ±7.82% ±10.24% 23:20:26 http/check_is_http_token.js n=1000000 key='Expires' 0.90 % ±3.20% ±4.26% ±5.56% 23:20:26 http/check_is_http_token.js n=1000000 key='Keep-Alive' 1.57 % ±3.52% ±4.69% ±6.10% 23:20:26 http/check_is_http_token.js n=1000000 key='Last-Modified' -1.16 % ±4.07% ±5.42% ±7.05% 23:20:26 http/check_is_http_token.js n=1000000 key='location' -3.06 % ±4.15% ±5.52% ±7.19% 23:20:26 http/check_is_http_token.js n=1000000 key='server' 2.93 % ±4.61% ±6.15% ±8.03% 23:20:26 http/check_is_http_token.js n=1000000 key='Server' 1.74 % ±3.64% ±4.85% ±6.31% 23:20:26 http/check_is_http_token.js n=1000000 key='status' 1.05 % ±3.68% ±4.89% ±6.37% 23:20:26 http/check_is_http_token.js n=1000000 key='TCN' 4.20 % ±5.12% ±6.82% ±8.88% 23:20:26 http/check_is_http_token.js n=1000000 key='Transfer-Encoding' -1.33 % ±4.85% ±6.46% ±8.41% 23:20:26 http/check_is_http_token.js n=1000000 key='Vary' -0.23 % ±4.29% ±5.71% ±7.44% 23:20:26 http/check_is_http_token.js n=1000000 key='version' -0.26 % ±3.68% ±4.90% ±6.38% 23:20:26 http/check_is_http_token.js n=1000000 key='x-frame-options' 0.57 % ±2.54% ±3.38% ±4.40% 23:20:26 http/check_is_http_token.js n=1000000 key='x-xss-protection' -1.27 % ±4.81% ±6.40% ±8.33% 23:20:26 http/check_is_http_token.js n=1000000 key='中文呢' -3.24 % ±6.48% ±8.62% ±11.24% 23:20:26 http/chunked.js duration=5 c=100 len=1 n=16 benchmarker='wrk' 0.45 % ±0.47% ±0.63% ±0.82% 23:20:26 http/chunked.js duration=5 c=100 len=1 n=1 benchmarker='wrk' 0.04 % ±0.07% ±0.09% ±0.11% 23:20:26 http/chunked.js duration=5 c=100 len=1 n=4 benchmarker='wrk' 0.12 % ±0.15% ±0.20% ±0.26% 23:20:26 http/chunked.js duration=5 c=100 len=1 n=8 benchmarker='wrk' 0.11 % ±0.23% ±0.31% ±0.41% 23:20:26 http/chunked.js duration=5 c=100 len=256 n=16 benchmarker='wrk' ** 1.07 % ±0.70% ±0.93% ±1.22% 23:20:26 http/chunked.js duration=5 c=100 len=256 n=1 benchmarker='wrk' 0.01 % ±0.05% ±0.07% ±0.09% 23:20:26 http/chunked.js duration=5 c=100 len=256 n=4 benchmarker='wrk' 0.03 % ±0.14% ±0.18% ±0.24% 23:20:26 http/chunked.js duration=5 c=100 len=256 n=8 benchmarker='wrk' 0.16 % ±0.25% ±0.33% ±0.43% 23:20:26 http/chunked.js duration=5 c=100 len=64 n=16 benchmarker='wrk' 0.78 % ±0.86% ±1.14% ±1.49% 23:20:26 http/chunked.js duration=5 c=100 len=64 n=1 benchmarker='wrk' 0.00 % ±0.08% ±0.10% ±0.13% 23:20:26 http/chunked.js duration=5 c=100 len=64 n=4 benchmarker='wrk' 0.06 % ±0.12% ±0.15% ±0.20% 23:20:26 http/chunked.js duration=5 c=100 len=64 n=8 benchmarker='wrk' -0.02 % ±0.26% ±0.35% ±0.46% 23:20:26 http/client-request-body.js method='end' len=1024 type='asc' dur=5 -0.28 % ±7.69% ±10.25% ±13.38% 23:20:26 http/client-request-body.js method='end' len=1024 type='buf' dur=5 0.63 % ±8.70% ±11.58% ±15.07% 23:20:26 http/client-request-body.js method='end' len=1024 type='utf' dur=5 -1.36 % ±7.72% ±10.28% ±13.39% 23:20:26 http/client-request-body.js method='end' len=256 type='asc' dur=5 -3.93 % ±8.14% ±10.83% ±14.11% 23:20:26 http/client-request-body.js method='end' len=256 type='buf' dur=5 4.35 % ±9.08% ±12.08% ±15.73% 23:20:26 http/client-request-body.js method='end' len=256 type='utf' dur=5 4.40 % ±8.27% ±11.00% ±14.33% 23:20:26 http/client-request-body.js method='end' len=32 type='asc' dur=5 1.28 % ±8.64% ±11.50% ±14.98% 23:20:26 http/client-request-body.js method='end' len=32 type='buf' dur=5 2.35 % ±9.70% ±12.90% ±16.79% 23:20:26 http/client-request-body.js method='end' len=32 type='utf' dur=5 0.75 % ±9.55% ±12.71% ±16.56% 23:20:26 http/client-request-body.js method='write' len=1024 type='asc' dur=5 -0.52 % ±8.16% ±10.86% ±14.14% 23:20:26 http/client-request-body.js method='write' len=1024 type='buf' dur=5 3.41 % ±8.53% ±11.35% ±14.78% 23:20:26 http/client-request-body.js method='write' len=1024 type='utf' dur=5 1.03 % ±8.23% ±10.94% ±14.24% 23:20:26 http/client-request-body.js method='write' len=256 type='asc' dur=5 2.44 % ±10.11% ±13.46% ±17.52% 23:20:26 http/client-request-body.js method='write' len=256 type='buf' dur=5 -0.57 % ±8.72% ±11.60% ±15.09% 23:20:26 http/client-request-body.js method='write' len=256 type='utf' dur=5 -1.64 % ±8.41% ±11.19% ±14.58% 23:20:26 http/client-request-body.js method='write' len=32 type='asc' dur=5 3.12 % ±8.32% ±11.07% ±14.42% 23:20:26 http/client-request-body.js method='write' len=32 type='buf' dur=5 3.76 % ±9.14% ±12.16% ±15.83% 23:20:26 http/client-request-body.js method='write' len=32 type='utf' dur=5 3.15 % ±9.44% ±12.56% ±16.34% 23:20:26 http/cluster.js duration=5 c=500 len=102400 type='buffer' benchmarker='wrk' 0.60 % ±3.13% ±4.17% ±5.43% 23:20:26 http/cluster.js duration=5 c=500 len=102400 type='bytes' benchmarker='wrk' 2.21 % ±3.22% ±4.30% ±5.61% 23:20:26 http/cluster.js duration=5 c=500 len=1024 type='buffer' benchmarker='wrk' 0.17 % ±3.23% ±4.30% ±5.59% 23:20:26 http/cluster.js duration=5 c=500 len=1024 type='bytes' benchmarker='wrk' 0.35 % ±3.28% ±4.36% ±5.68% 23:20:26 http/cluster.js duration=5 c=500 len=4 type='buffer' benchmarker='wrk' -0.60 % ±3.42% ±4.55% ±5.92% 23:20:26 http/cluster.js duration=5 c=500 len=4 type='bytes' benchmarker='wrk' 1.82 % ±2.31% ±3.08% ±4.01% 23:20:26 http/cluster.js duration=5 c=50 len=102400 type='buffer' benchmarker='wrk' * 4.94 % ±3.74% ±4.98% ±6.48% 23:20:26 http/cluster.js duration=5 c=50 len=102400 type='bytes' benchmarker='wrk' -2.31 % ±3.62% ±4.81% ±6.27% 23:20:26 http/cluster.js duration=5 c=50 len=1024 type='buffer' benchmarker='wrk' -3.97 % ±5.67% ±7.55% ±9.82% 23:20:26 http/cluster.js duration=5 c=50 len=1024 type='bytes' benchmarker='wrk' 1.26 % ±6.17% ±8.21% ±10.70% 23:20:26 http/cluster.js duration=5 c=50 len=4 type='buffer' benchmarker='wrk' -3.58 % ±5.97% ±7.94% ±10.33% 23:20:26 http/cluster.js duration=5 c=50 len=4 type='bytes' benchmarker='wrk' -0.53 % ±5.66% ±7.53% ±9.80% 23:20:26 http/create-clientrequest.js e=1 arg='options' url='idn' -0.25 % ±8.27% ±11.00% ±14.32% 23:20:26 http/create-clientrequest.js e=1 arg='options' url='long' -5.66 % ±7.02% ±9.33% ±12.15% 23:20:26 http/create-clientrequest.js e=1 arg='options' url='wpt' -2.20 % ±7.11% ±9.46% ±12.31% 23:20:26 http/create-clientrequest.js e=1 arg='string' url='idn' 4.12 % ±6.83% ±9.11% ±11.89% 23:20:26 http/create-clientrequest.js e=1 arg='string' url='long' -3.20 % ±9.09% ±12.09% ±15.75% 23:20:26 http/create-clientrequest.js e=1 arg='string' url='wpt' -0.84 % ±5.77% ±7.68% ±9.99% 23:20:26 http/create-clientrequest.js e=1 arg='URL' url='idn' 1.91 % ±6.31% ±8.40% ±10.96% 23:20:26 http/create-clientrequest.js e=1 arg='URL' url='long' 0.03 % ±7.48% ±9.95% ±12.96% 23:20:26 http/create-clientrequest.js e=1 arg='URL' url='wpt' 1.19 % ±6.10% ±8.11% ±10.56% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='asc' benchmarker='wrk' -0.70 % ±2.56% ±3.41% ±4.45% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='buf' benchmarker='wrk' -2.56 % ±8.07% ±10.73% ±13.97% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=1048576 type='utf' benchmarker='wrk' 0.14 % ±1.98% ±2.64% ±3.44% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='asc' benchmarker='wrk' -0.81 % ±2.10% ±2.80% ±3.66% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='buf' benchmarker='wrk' -0.03 % ±2.37% ±3.15% ±4.10% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=131072 type='utf' benchmarker='wrk' 0.45 % ±1.49% ±1.99% ±2.59% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='asc' benchmarker='wrk' -0.52 % ±1.53% ±2.04% ±2.66% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='buf' benchmarker='wrk' * 5.79 % ±4.40% ±5.86% ±7.64% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=262144 type='utf' benchmarker='wrk' -0.01 % ±2.32% ±3.08% ±4.01% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='asc' benchmarker='wrk' 0.23 % ±2.33% ±3.10% ±4.04% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='buf' benchmarker='wrk' 1.37 % ±2.84% ±3.78% ±4.92% 23:20:26 http/end-vs-write-end.js duration=5 method='end' c=100 len=65536 type='utf' benchmarker='wrk' * -1.82 % ±1.80% ±2.40% ±3.12% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='asc' benchmarker='wrk' 0.10 % ±5.13% ±6.83% ±8.89% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='buf' benchmarker='wrk' 2.24 % ±7.30% ±9.72% ±12.66% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=1048576 type='utf' benchmarker='wrk' -0.21 % ±4.83% ±6.43% ±8.37% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='asc' benchmarker='wrk' -0.11 % ±3.13% ±4.16% ±5.41% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='buf' benchmarker='wrk' 0.90 % ±2.47% ±3.29% ±4.28% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=131072 type='utf' benchmarker='wrk' -1.88 % ±3.44% ±4.57% ±5.96% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='asc' benchmarker='wrk' 2.08 % ±4.64% ±6.17% ±8.04% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='buf' benchmarker='wrk' -3.69 % ±4.93% ±6.56% ±8.54% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=262144 type='utf' benchmarker='wrk' 2.09 % ±4.51% ±6.01% ±7.82% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='asc' benchmarker='wrk' -0.63 % ±1.89% ±2.51% ±3.27% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='buf' benchmarker='wrk' * 2.42 % ±1.91% ±2.54% ±3.30% 23:20:26 http/end-vs-write-end.js duration=5 method='write' c=100 len=65536 type='utf' benchmarker='wrk' -0.89 % ±2.05% ±2.72% ±3.55% 23:20:26 http/headers.js duration=5 len=100 n=1000 benchmarker='wrk' 2.03 % ±2.67% ±3.55% ±4.63% 23:20:26 http/headers.js duration=5 len=100 n=10 benchmarker='wrk' 4.50 % ±5.24% ±6.98% ±9.09% 23:20:26 http/headers.js duration=5 len=1 n=1000 benchmarker='wrk' 1.56 % ±2.33% ±3.11% ±4.04% 23:20:26 http/headers.js duration=5 len=1 n=10 benchmarker='wrk' 1.22 % ±5.07% ±6.75% ±8.81% 23:20:26 ```

@OrKoN I don’t think so. But if it only fires in the timeout case, that should be okay.

@OrKoN Since you’d be return;ing here either way, whether the call threw an exception or not, I don’t think so. You can wrap the call in USE() to make it explicit that you’re discarding the return value because of that.

lgtm

CI: https://ci.nodejs.org/job/node-test-pull-request/30220/

Co-Authored-By: Anna Henningsen <github@addaleax.net>

CI: https://ci.nodejs.org/job/node-test-pull-request/30257/

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. PR-URL: #32329 Fixes: #27363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Landed in b149eef

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. PR-URL: #32329 Fixes: #27363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@BethGriggs this should ideally be backported down to 10 in the next patch release. When is that scheduled? I assume this is going to need backport PRs, should we open them already?

The next v12.x is due out 28th April and the next v10.x May 5th. Assuming that this commit goes out with v13.x current (scheduled next Tuesday), then this change should be able to make it into the upcoming releases. Opening backport PRs anytime after it has gone out in current (next Tuesday) makes sense to me.

@richardlau Should I create one more backport PR for 10.x? My assumption was that it should be in 12.x for a while first.

@Flarna I've been going around the lts-watch-v10.x labelled PRs to see what's landable for the upcoming 10.x release I'm preparing. Your assumption is correct that usually we'd want things to flow back in an ordered manner (so 12.x before 10.x). I haven't checked to see if the 12.x PR would land cleanly on 10.x. If it does we can add a lts-watch-v10.x label to the 12.x backport PR, otherwise yes please open a backport PR for 10.x so it can be ready for a future 10.x release (there's no regular schedule for non-security 10.x releases as it is now in maintenance -- the release team will review periodically to see if there is enough content to warrant a release).

I did a fast check and it seems that the commit for 12.x doesn't land on 10.x so seems a dedicated backport PR will be needed. I may create one once the backport to 12.x is finished.

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. Backport-PR-URL: #34131 PR-URL: #32329 Fixes: #27363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@OrKoN Could this change cause the following issue with uploading?
#35661

PR-URL: nodejs#36630 Refs: nodejs#32329 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #36630 Refs: #32329 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36630 Refs: nodejs#32329 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>

This reverts commit 9156f43. PR-URL: #36890 Refs: #32329 Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>

PR-URL: #36630 Backport-PR-URL: #36890 Refs: #32329 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #36630 Refs: #32329 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>

has this been backported to v14?

…46052) Resolves #39689, partially resolves #28642 (see notes below) Inspired by #44627 In #28642 it was also asked to expose `server.headersTimeout`, but it is probably not needed for most use cases and not implemented even in `next start`. It was needed to change this option before nodejs/node#27363. There also exists a rare bug that is described here nodejs/node#32329 (comment). To fix this exposing `server.headersTimeout` might be required both in `server.js` and in `next start`. Co-authored-by: JJ Kasper <jj@jjsweb.site>

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. PR-URL: nodejs/node#32329 Fixes: nodejs/node#27363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. Backport-PR-URL: nodejs/node#34131 PR-URL: nodejs/node#32329 Fixes: nodejs/node#27363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

OrKoN added 2 commits March 17, 2020 18:10

http: fix incorrect headersTimeout measurement

ed7b316

For keep-alive connections, the headersTimeout may fire during subsequent request because the measurement was reset after a request and not before a request. Fixes: nodejs#27363

Clean up

f162c42

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Mar 17, 2020

OrKoN commented Mar 17, 2020

View reviewed changes

OrKoN mentioned this pull request Mar 19, 2020

http: fix incorrect headersTimeout measurement #30184

Closed

3 tasks

OrKoN changed the title http: fix incorrect headersTimeout measurement http: fix incorrect headersTimeout measurement (alt) Mar 19, 2020

Update async_hooks test

2d91c30

addaleax approved these changes Mar 29, 2020

View reviewed changes

addaleax added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. review wanted PRs that need reviews. labels Mar 29, 2020

mcollina approved these changes Mar 29, 2020

View reviewed changes

Update src/node_http_parser.cc

9536c1b

Co-Authored-By: Anna Henningsen <github@addaleax.net>

mcollina closed this Apr 2, 2020

mcollina added lts-watch-v10.x labels Apr 9, 2020

richardlau added the backport-requested-v10.x label Jul 2, 2020

Flarna added the backport-open-v12.x label Jul 10, 2020

Flarna added backported-to-v12.x and removed backport-open-v12.x labels Aug 18, 2020

codebytere mentioned this pull request Sep 28, 2020

v12.19.0 proposal #35401

Merged

andrewnester mentioned this pull request Oct 20, 2020

HTTP: Connection resets after 60 seconds in node.js upload application #35661

Closed

MylesBorins removed the lts-watch-v12.x label Nov 16, 2020

Lxxyx mentioned this pull request Dec 25, 2020

lib: remove dead code from internal/http.js #36630

Merged

2 tasks

aduh95 mentioned this pull request Jan 12, 2021

[Backport v15.x] http: remove dead code from internal/http.js #36890

Closed

danielleadams pushed a commit that referenced this pull request Jan 13, 2021

Revert "http: remove dead code from internal/http.js"

7e3ad1b

This reverts commit 9156f43. PR-URL: #36890 Refs: #32329 Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>

AdamKatzDev mentioned this pull request Feb 17, 2023

feat(standalone): allow configuring KEEP_ALIVE_TIMEOUT via env var vercel/next.js#46052

Merged

codyebberson mentioned this pull request Jun 19, 2023

Fixes #2253 - increase server keep alive timeout medplum/medplum#2256

Merged

pimterry mentioned this pull request Aug 4, 2025

Increase default keepAliveTimeout to 65s #59203

Closed

                                   req.insecureHTTPParser === undefined ?
-                                    isLenient() : req.insecureHTTPParser);
+                                    isLenient() : req.insecureHTTPParser,
+);

-                  return;
-                }
+              function onParserTimeout(server, socket) {
+                const serverTimeout = server.emit('timeout', socket);

+                      if (!cb->IsFunction())
+                        return;
+                      MakeCallback(cb.As<Function>(), 0, nullptr);

+                    auto parsing_time = (now - header_parsing_start_time_) / 1e6;
+                    if (parsing_time > headers_timeout_) {
+                      Local<Value> cb =

http: fix incorrect headersTimeout measurement (alt) by OrKoN · Pull Request #32329 · nodejs/node · GitHub

Uh oh!

Uh oh!

http: fix incorrect headersTimeout measurement (alt) - #32329

http: fix incorrect headersTimeout measurement (alt)#32329OrKoN wants to merge 4 commits intonodejs:masternodejs/node:masterfrom OrKoN:fix-headers-timeout-cppOrKoN/node:fix-headers-timeout-cppCopy head branch name to clipboard

Conversation

OrKoN commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OrKoN commented Mar 17, 2020

Uh oh!

OrKoN commented Mar 19, 2020

Uh oh!

mcollina commented Mar 19, 2020

Uh oh!

mcollina commented Mar 19, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 29, 2020

Uh oh!

nodejs-github-bot commented Mar 30, 2020

Uh oh!

mcollina commented Apr 2, 2020

Uh oh!

mcollina commented Apr 10, 2020

Uh oh!

BethGriggs commented Apr 10, 2020

Uh oh!

Flarna commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardlau commented Jul 2, 2020

Uh oh!

Flarna commented Jul 2, 2020

Uh oh!

andrewnester commented Oct 20, 2020

Uh oh!

ArnoldZokas commented Jun 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

http: fix incorrect headersTimeout measurement (alt)#32329
OrKoN wants to merge 4 commits into
nodejs:masterfrom
OrKoN:fix-headers-timeout-cpp

OrKoN commented Mar 17, 2020 •
edited
Loading

Flarna commented Jul 2, 2020 •
edited
Loading