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

fix: handle request abort correctly #1790

Merged
merged 1 commit into from
Nov 11, 2024
Merged

fix: handle request abort correctly #1790

merged 1 commit into from
Nov 11, 2024

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Nov 11, 2024

  • Call Response.body's cancel method when the request is aborted on the client with AbortSignal in fetch
  • Call the piped stream's cancel method when piped;
const res = new Response(
  foo.pipe(bar)
)
// And when res is cancelled somehow, foo's cancel method should be implemented to dispose the original stream correctly
  • Test and compare the behaviors in both native Node and ponyfill implementations

Copy link
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@whatwg-node/node-fetch 0.7.2-alpha-20241111123417-4e4d48a16cce52587b1a06b11e72803047cc473d npm ↗︎ unpkg ↗︎
@whatwg-node/server 0.9.54-alpha-20241111123417-4e4d48a16cce52587b1a06b11e72803047cc473d npm ↗︎ unpkg ↗︎

Copy link
Contributor

@benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=139.833159 min=13     med=140     max=196     p(90)=161     p(95)=165    
     data_received..................: 22 MB  730 kB/s
     data_sent......................: 14 MB  468 kB/s
     http_req_blocked...............: avg=3.03µs     min=601ns  med=1.17µs  max=14.53ms p(90)=1.89µs  p(95)=2.13µs 
     http_req_connecting............: avg=1.31µs     min=0s     med=0s      max=5.05ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=20.89ms    min=2.06ms med=20.21ms max=1.1s    p(90)=26.31ms p(95)=28.04ms
       { expected_response:true }...: avg=20.89ms    min=2.06ms med=20.21ms max=1.1s    p(90)=26.31ms p(95)=28.04ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 143096
     http_req_receiving.............: avg=34.4µs     min=8.87µs med=22.81µs max=20.4ms  p(90)=37.23µs p(95)=43.59µs
     http_req_sending...............: avg=10.68µs    min=2.95µs med=5.63µs  max=27.27ms p(90)=9.2µs   p(95)=12.41µs
     http_req_tls_handshaking.......: avg=0s         min=0s     med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=20.85ms    min=2.03ms med=20.17ms max=1.1s    p(90)=26.27ms p(95)=27.97ms
     http_reqs......................: 143096 4769.364283/s
     iteration_duration.............: avg=41.89ms    min=7.66ms med=40.33ms max=1.12s   p(90)=46.36ms p(95)=51.41ms
     iterations.....................: 71535  2384.248854/s
     vus............................: 97     min=97        max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

@benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 233702      ✗ 0     
     data_received..................: 24 MB   783 kB/s
     data_sent......................: 9.3 MB  312 kB/s
     http_req_blocked...............: avg=1.48µs   min=971ns    med=1.26µs   max=178.05µs p(90)=1.98µs   p(95)=2.17µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=120.83µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=195.05µs min=143.64µs med=181.6µs  max=25.1ms   p(90)=207.1µs  p(95)=220.46µs
       { expected_response:true }...: avg=195.05µs min=143.64µs med=181.6µs  max=25.1ms   p(90)=207.1µs  p(95)=220.46µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 116851
     http_req_receiving.............: avg=25.46µs  min=13.81µs  med=24.12µs  max=1.44ms   p(90)=31.05µs  p(95)=33.03µs 
     http_req_sending...............: avg=6.44µs   min=4.2µs    med=5.72µs   max=1.47ms   p(90)=8.27µs   p(95)=8.85µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=163.14µs min=118.78µs med=149.25µs max=24.32ms  p(90)=171.76µs p(95)=184.33µs
     http_reqs......................: 116851  3894.909168/s
     iteration_duration.............: avg=252.31µs min=196.37µs med=238.19µs max=25.29ms  p(90)=266.92µs p(95)=283.21µs
     iterations.....................: 116851  3894.909168/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

@benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 220400      ✗ 0     
     data_received..................: 22 MB   738 kB/s
     data_sent......................: 8.8 MB  294 kB/s
     http_req_blocked...............: avg=1.46µs   min=891ns    med=1.26µs   max=206.97µs p(90)=1.97µs   p(95)=2.16µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=142.45µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=210.24µs min=153.43µs med=196.16µs max=46.8ms   p(90)=222.81µs p(95)=237.32µs
       { expected_response:true }...: avg=210.24µs min=153.43µs med=196.16µs max=46.8ms   p(90)=222.81µs p(95)=237.32µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 110200
     http_req_receiving.............: avg=25.64µs  min=13.8µs   med=24.39µs  max=4.7ms    p(90)=31.4µs   p(95)=33.33µs 
     http_req_sending...............: avg=6.46µs   min=4.06µs   med=5.78µs   max=808.41µs p(90)=8.3µs    p(95)=8.97µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=178.14µs min=127.66µs med=164µs    max=46.72ms  p(90)=187.16µs p(95)=200.51µs
     http_reqs......................: 110200  3673.055407/s
     iteration_duration.............: avg=267.74µs min=198.85µs med=252.68µs max=46.94ms  p(90)=283.59µs p(95)=302.73µs
     iterations.....................: 110200  3673.055407/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

@benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 294616      ✗ 0     
     data_received..................: 29 MB   967 kB/s
     data_sent......................: 12 MB   393 kB/s
     http_req_blocked...............: avg=1.39µs   min=852ns    med=1.19µs   max=174.72µs p(90)=1.92µs   p(95)=2.09µs  
     http_req_connecting............: avg=0ns      min=0s       med=0s       max=118.8µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=141.93µs min=94.99µs  med=136.07µs max=7.6ms    p(90)=157.98µs p(95)=165.21µs
       { expected_response:true }...: avg=141.93µs min=94.99µs  med=136.07µs max=7.6ms    p(90)=157.98µs p(95)=165.21µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 147308
     http_req_receiving.............: avg=24.51µs  min=12.34µs  med=23.43µs  max=1.22ms   p(90)=30.28µs  p(95)=32.47µs 
     http_req_sending...............: avg=6.27µs   min=4.08µs   med=5.58µs   max=110.88µs p(90)=8.1µs    p(95)=8.7µs   
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=111.13µs min=69.87µs  med=104.92µs max=7.55ms   p(90)=123.89µs p(95)=129.9µs 
     http_reqs......................: 147308  4910.054944/s
     iteration_duration.............: avg=199.27µs min=139.38µs med=192.64µs max=7.7ms    p(90)=217.54µs p(95)=226.99µs
     iterations.....................: 147308  4910.054944/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

@benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=139.95537 min=54      med=140     max=199      p(90)=161     p(95)=165    
     data_received..................: 23 MB  777 kB/s
     data_sent......................: 15 MB  503 kB/s
     http_req_blocked...............: avg=3.48µs    min=651ns   med=1.33µs  max=6.53ms   p(90)=1.99µs  p(95)=2.25µs 
     http_req_connecting............: avg=1.75µs    min=0s      med=0s      max=6.5ms    p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=19.61ms   min=1.6ms   med=19.02ms max=909.57ms p(90)=25.03ms p(95)=27.18ms
       { expected_response:true }...: avg=19.61ms   min=1.6ms   med=19.02ms max=909.57ms p(90)=25.03ms p(95)=27.18ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 152414
     http_req_receiving.............: avg=32.32µs   min=9.05µs  med=22.35µs max=27.27ms  p(90)=37.02µs p(95)=44µs   
     http_req_sending...............: avg=10.42µs   min=3.4µs   med=6.19µs  max=20.45ms  p(90)=9.25µs  p(95)=12.99µs
     http_req_tls_handshaking.......: avg=0s        min=0s      med=0s      max=0s       p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=19.57ms   min=1.54ms  med=18.98ms max=909.49ms p(90)=24.99ms p(95)=27.1ms 
     http_reqs......................: 152414 5079.444919/s
     iteration_duration.............: avg=39.34ms   min=16.98ms med=37.92ms max=931.95ms p(90)=44.34ms p(95)=49.19ms
     iterations.....................: 76182  2538.889294/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

@ardatan ardatan merged commit c7d49b1 into master Nov 11, 2024
25 checks passed
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.

1 participant