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

Test if it leaks when the given signal never aborts and connection succeeds #1947

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Jan 7, 2025

This PR adds two tests;

  • Adds an AbortSignal from an AbortController never aborts, and the fetch call succeeds to see if it leaks
  • Adds an AbortSignal.timeout and it doesn't timeout and fetch call succeeds before it times out. And we check if it leaks

And it also removes the event listener added in node-libcurl based fetch implementation to make sure it doesn't leak.
This change doesn't affect Hive Gateway's this issue graphql-hive/gateway#303 (comment) because Hive GW doesn't use node-libcurl directly. Only tests are added to see if the leak is reproduced regardless the change.

Copy link
Contributor

github-actions bot commented Jan 7, 2025

🚀 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/fetch 0.10.2-alpha-20250107153812-2b7cc1f8c195a23bdaa804e863be3032890ab3c1 npm ↗︎ unpkg ↗︎
@whatwg-node/node-fetch 0.7.6-alpha-20250107153812-2b7cc1f8c195a23bdaa804e863be3032890ab3c1 npm ↗︎ unpkg ↗︎

Copy link
Contributor

github-actions bot commented Jan 7, 2025

@benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=140.587593 min=32      med=142     max=183     p(90)=159     p(95)=165    
     data_received..................: 21 MB  693 kB/s
     data_sent......................: 13 MB  444 kB/s
     http_req_blocked...............: avg=3.58µs     min=632ns   med=1.33µs  max=5.57ms  p(90)=2.05µs  p(95)=2.35µs 
     http_req_connecting............: avg=1.64µs     min=0s      med=0s      max=4.57ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=22ms       min=6.12ms  med=21.4ms  max=1.12s   p(90)=27.58ms p(95)=29.1ms 
       { expected_response:true }...: avg=22ms       min=6.12ms  med=21.4ms  max=1.12s   p(90)=27.58ms p(95)=29.1ms 
     http_req_failed................: 0.00%  ✓ 0           ✗ 135910
     http_req_receiving.............: avg=35.99µs    min=9.22µs  med=24.37µs max=31.45ms p(90)=38.63µs p(95)=44.97µs
     http_req_sending...............: avg=11.13µs    min=3.38µs  med=6.45µs  max=18.1ms  p(90)=10.06µs p(95)=14.09µs
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=21.95ms    min=6.09ms  med=21.36ms max=1.12s   p(90)=27.54ms p(95)=29.05ms
     http_reqs......................: 135910 4529.922872/s
     iteration_duration.............: avg=44.11ms    min=18.17ms med=42.6ms  max=1.14s   p(90)=47.54ms p(95)=53.55ms
     iterations.....................: 67928  2264.061517/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Jan 7, 2025

@benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=140.749353 min=24      med=141     max=181     p(90)=160     p(95)=166    
     data_received..................: 22 MB  722 kB/s
     data_sent......................: 14 MB  467 kB/s
     http_req_blocked...............: avg=3.74µs     min=631ns   med=1.42µs  max=5.07ms  p(90)=2.05µs  p(95)=2.31µs 
     http_req_connecting............: avg=1.87µs     min=0s      med=0s      max=4.99ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=21.12ms    min=4.22ms  med=20.49ms max=1.12s   p(90)=26.55ms p(95)=28ms   
       { expected_response:true }...: avg=21.12ms    min=4.22ms  med=20.49ms max=1.12s   p(90)=26.55ms p(95)=28ms   
     http_req_failed................: 0.00%  ✓ 0           ✗ 141533
     http_req_receiving.............: avg=35.89µs    min=9.56µs  med=23.97µs max=22.51ms p(90)=38.66µs p(95)=45.55µs
     http_req_sending...............: avg=10.74µs    min=3.47µs  med=6.87µs  max=5.4ms   p(90)=10.05µs p(95)=13.9µs 
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=21.07ms    min=4.08ms  med=20.46ms max=1.12s   p(90)=26.5ms  p(95)=27.94ms
     http_reqs......................: 141533 4717.26625/s
     iteration_duration.............: avg=42.36ms    min=17.75ms med=40.88ms max=1.15s   p(90)=46.15ms p(95)=51.98ms
     iterations.....................: 70753  2358.183173/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Jan 7, 2025

@benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 295874      ✗ 0     
     data_received..................: 29 MB   971 kB/s
     data_sent......................: 12 MB   395 kB/s
     http_req_blocked...............: avg=1.38µs   min=881ns   med=1.18µs   max=310.89µs p(90)=1.86µs   p(95)=2.02µs  
     http_req_connecting............: avg=0ns      min=0s      med=0s       max=115.1µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=139.94µs min=95.18µs med=135.24µs max=5.93ms   p(90)=157.35µs p(95)=164.53µs
       { expected_response:true }...: avg=139.94µs min=95.18µs med=135.24µs max=5.93ms   p(90)=157.35µs p(95)=164.53µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 147937
     http_req_receiving.............: avg=24.6µs   min=12.13µs med=23.13µs  max=2.98ms   p(90)=30.26µs  p(95)=33.01µs 
     http_req_sending...............: avg=6.31µs   min=4.07µs  med=5.46µs   max=581.19µs p(90)=8.09µs   p(95)=8.94µs  
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=109.01µs min=67.03µs med=103.87µs max=5.88ms   p(90)=123µs    p(95)=128.82µs
     http_reqs......................: 147937  4931.066716/s
     iteration_duration.............: avg=198.28µs min=142.7µs med=192.95µs max=6.11ms   p(90)=218.11µs p(95)=227.18µs
     iterations.....................: 147937  4931.066716/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Jan 7, 2025

@benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 217646      ✗ 0     
     data_received..................: 22 MB   729 kB/s
     data_sent......................: 8.7 MB  290 kB/s
     http_req_blocked...............: avg=1.45µs   min=902ns    med=1.22µs   max=179.53µs p(90)=1.94µs   p(95)=2.13µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=119.41µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=212.69µs min=153.94µs med=201.08µs max=55.81ms  p(90)=226.75µs p(95)=236.09µs
       { expected_response:true }...: avg=212.69µs min=153.94µs med=201.08µs max=55.81ms  p(90)=226.75µs p(95)=236.09µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 108823
     http_req_receiving.............: avg=25.86µs  min=13.68µs  med=24.45µs  max=2.64ms   p(90)=31.25µs  p(95)=33.91µs 
     http_req_sending...............: avg=6.43µs   min=3.98µs   med=5.61µs   max=2.84ms   p(90)=8.23µs   p(95)=9.01µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=180.39µs min=129.59µs med=169.01µs max=55.73ms  p(90)=191.68µs p(95)=200.24µs
     http_reqs......................: 108823  3627.308049/s
     iteration_duration.............: avg=271.18µs min=199.32µs med=258.37µs max=55.96ms  p(90)=288.09µs p(95)=299.98µs
     iterations.....................: 108823  3627.308049/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Jan 7, 2025

@benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 206850      ✗ 0     
     data_received..................: 21 MB   693 kB/s
     data_sent......................: 8.3 MB  276 kB/s
     http_req_blocked...............: avg=1.42µs   min=891ns    med=1.22µs   max=216.79µs p(90)=1.9µs    p(95)=2.06µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=144.3µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=226.64µs min=170.9µs  med=216.16µs max=11.58ms  p(90)=243.49µs p(95)=254.52µs
       { expected_response:true }...: avg=226.64µs min=170.9µs  med=216.16µs max=11.58ms  p(90)=243.49µs p(95)=254.52µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 103425
     http_req_receiving.............: avg=25.41µs  min=13.54µs  med=23.79µs  max=1.79ms   p(90)=30.78µs  p(95)=33.47µs 
     http_req_sending...............: avg=6.45µs   min=3.97µs   med=5.72µs   max=232.19µs p(90)=8.18µs   p(95)=8.96µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=194.78µs min=147.82µs med=184.13µs max=11.52ms  p(90)=208.77µs p(95)=218.98µs
     http_reqs......................: 103425  3447.356173/s
     iteration_duration.............: avg=285.58µs min=214.5µs  med=274.28µs max=12.1ms   p(90)=304.65µs p(95)=318.08µs
     iterations.....................: 103425  3447.356173/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

@dotansimha dotansimha requested a review from Urigo January 8, 2025 08:03
@ardatan ardatan merged commit 9b39c3e into master Jan 9, 2025
24 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.

3 participants