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

Run tests with all Node frameworks & some fixes on URL and ReadableStream.cancel #1929

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Dec 27, 2024

Improvements on tests

  • Run tests with all possible server implementation(Node frameworks; uWS, Express, Fastify, Koa, Hapi and vanilla node:http)
  • Enable Leak tests for all test cases again for all frameworks thanks to our more stable leak detector

Fixes on `node-fetch

  • Normalization fixes on URL (not related directly, realized during tests)

Normalize the URL if it has a port and query string without a pathname;

+ http://example.com:80?query
- http://example.com:80/?query

Previously, it was normalized like below which was incorrect;

- http://example.com:80?query
+ http://example.com/:80?query
  • Fix URL.origin (not related directly, realized during tests)

When the URL has a port, origin was doubling the port number;

- http://example.com:80:80
+ http://example.com:80
  • Fix ReadableStream[Symbol.asyncIterator] (realized when using Readable.from in hapi integration)

ReadableStream uses Readable so it uses Symbol.asyncIterator method of Readable but the returned iterator's .return method doesn't handle cancellation correctly. So we need to call readable.destroy(optionalError) manually to cancel the stream.

This allows ReadableStream to use implementations relying on AsyncIterable.cancel to handle cancellation like Readable.from

Previously the following was not handling cancellation;

const res = new ReadableStream({
  start(controller) {
    controller.enqueue('Hello');
    controller.enqueue('World');
  },
  cancel(reason) {
    console.log('cancelled', reason);
  }
});

const readable = Readable.from(res);

readable.destroy(new Error('MY REASON'));

// Should log 'cancelled MY REASON'

Copy link
Contributor

github-actions bot commented Dec 27, 2024

🚀 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.6-alpha-20241227133443-491da4e9c34920079611c68a2aa3b9613ac93caa npm ↗︎ unpkg ↗︎

Copy link
Contributor

github-actions bot commented Dec 27, 2024

@benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=139.768604 min=13      med=140     max=190     p(90)=160     p(95)=165    
     data_received..................: 22 MB  722 kB/s
     data_sent......................: 14 MB  463 kB/s
     http_req_blocked...............: avg=2.09µs     min=641ns   med=1.26µs  max=4.33ms  p(90)=2.03µs  p(95)=2.28µs 
     http_req_connecting............: avg=239ns      min=0s      med=0s      max=3.11ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=21.11ms    min=3.44ms  med=20.41ms max=1.13s   p(90)=26.33ms p(95)=27.84ms
       { expected_response:true }...: avg=21.11ms    min=3.44ms  med=20.41ms max=1.13s   p(90)=26.33ms p(95)=27.84ms
     http_req_failed................: 0.00%  ✓ 10          ✗ 141631
     http_req_receiving.............: avg=32.34µs    min=9.22µs  med=23.1µs  max=18.34ms p(90)=37.67µs p(95)=44.12µs
     http_req_sending...............: avg=10.24µs    min=3.41µs  med=5.91µs  max=16.66ms p(90)=9.69µs  p(95)=13.44µ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=3.41ms  med=20.37ms max=1.13s   p(90)=26.29ms p(95)=27.77ms
     http_reqs......................: 141641 4720.588521/s
     iteration_duration.............: avg=42.33ms    min=10.82ms med=40.73ms max=1.15s   p(90)=46.51ms p(95)=51.97ms
     iterations.....................: 70792  2359.344417/s
     vus............................: 2      min=2         max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Dec 27, 2024

@benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=140.122108 min=12     med=141     max=191     p(90)=160     p(95)=165    
     data_received..................: 23 MB  764 kB/s
     data_sent......................: 15 MB  495 kB/s
     http_req_blocked...............: avg=3.26µs     min=652ns  med=1.35µs  max=13.1ms  p(90)=2.02µs  p(95)=2.32µs 
     http_req_connecting............: avg=1.26µs     min=0s     med=0s      max=5.21ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=19.95ms    min=2.93ms med=19.37ms max=1.07s   p(90)=25.03ms p(95)=26.47ms
       { expected_response:true }...: avg=19.95ms    min=2.93ms med=19.37ms max=1.07s   p(90)=25.03ms p(95)=26.47ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 149849
     http_req_receiving.............: avg=33.27µs    min=9.17µs med=22.19µs max=18.43ms p(90)=36.43µs p(95)=43.95µs
     http_req_sending...............: avg=11.16µs    min=3.39µs med=6.28µs  max=11.45ms p(90)=9.41µs  p(95)=13.88µs
     http_req_tls_handshaking.......: avg=0s         min=0s     med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=19.91ms    min=2.89ms med=19.34ms max=1.07s   p(90)=24.99ms p(95)=26.41ms
     http_reqs......................: 149849 4993.957132/s
     iteration_duration.............: avg=40.01ms    min=9.74ms med=38.56ms max=1.09s   p(90)=44.1ms  p(95)=49.13ms
     iterations.....................: 74901  2496.195391/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Dec 27, 2024

@benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 294460      ✗ 0     
     data_received..................: 29 MB   967 kB/s
     data_sent......................: 12 MB   393 kB/s
     http_req_blocked...............: avg=1.39µs   min=842ns    med=1.16µs   max=295.59µs p(90)=1.88µs   p(95)=2.05µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=152.56µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=140.49µs min=92.72µs  med=135.59µs max=5.79ms   p(90)=157.66µs p(95)=164.78µs
       { expected_response:true }...: avg=140.49µs min=92.72µs  med=135.59µs max=5.79ms   p(90)=157.66µs p(95)=164.78µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 147230
     http_req_receiving.............: avg=24.69µs  min=12.38µs  med=23.1µs   max=3.19ms   p(90)=30.24µs  p(95)=33.08µs 
     http_req_sending...............: avg=6.34µs   min=3.9µs    med=5.49µs   max=232.25µs p(90)=8.16µs   p(95)=9µs     
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=109.45µs min=68.12µs  med=104.23µs max=5.62ms   p(90)=123.44µs p(95)=129.43µs
     http_reqs......................: 147230  4907.476945/s
     iteration_duration.............: avg=199.2µs  min=141.69µs med=193.49µs max=6.07ms   p(90)=219.01µs p(95)=228.65µs
     iterations.....................: 147230  4907.476945/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Dec 27, 2024

@benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 215212      ✗ 0     
     data_received..................: 22 MB   721 kB/s
     data_sent......................: 8.6 MB  287 kB/s
     http_req_blocked...............: avg=1.43µs   min=871ns    med=1.2µs    max=191.93µs p(90)=1.9µs    p(95)=2.06µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=131.61µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=214.81µs min=154.94µs med=203.01µs max=55.59ms  p(90)=229.19µs p(95)=238.19µs
       { expected_response:true }...: avg=214.81µs min=154.94µs med=203.01µs max=55.59ms  p(90)=229.19µs p(95)=238.19µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 107606
     http_req_receiving.............: avg=26.19µs  min=13.69µs  med=24.56µs  max=2.89ms   p(90)=31.57µs  p(95)=34.18µs 
     http_req_sending...............: avg=6.53µs   min=4.1µs    med=5.8µs    max=1.04ms   p(90)=8.22µs   p(95)=9.06µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=182.08µs min=131.09µs med=170.62µs max=55.52ms  p(90)=193.34µs p(95)=201.48µs
     http_reqs......................: 107606  3586.723654/s
     iteration_duration.............: avg=274.11µs min=202.09µs med=261.09µs max=55.7ms   p(90)=291.78µs p(95)=303.75µs
     iterations.....................: 107606  3586.723654/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Dec 27, 2024

@benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 207294      ✗ 0     
     data_received..................: 21 MB   694 kB/s
     data_sent......................: 8.3 MB  276 kB/s
     http_req_blocked...............: avg=1.42µs   min=891ns    med=1.2µs    max=297.3µs  p(90)=1.91µs   p(95)=2.08µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=120.19µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=225.74µs min=172.24µs med=215.03µs max=7.41ms   p(90)=242.24µs p(95)=252.63µs
       { expected_response:true }...: avg=225.74µs min=172.24µs med=215.03µs max=7.41ms   p(90)=242.24µs p(95)=252.63µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 103647
     http_req_receiving.............: avg=25.49µs  min=13.61µs  med=23.75µs  max=3.07ms   p(90)=30.74µs  p(95)=33.54µs 
     http_req_sending...............: avg=6.38µs   min=3.99µs   med=5.64µs   max=285.04µs p(90)=8.11µs   p(95)=8.79µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=193.86µs min=146.78µs med=183.13µs max=7.37ms   p(90)=207.51µs p(95)=217.12µs
     http_reqs......................: 103647  3454.770329/s
     iteration_duration.............: avg=284.97µs min=215.4µs  med=273.51µs max=7.48ms   p(90)=303.83µs p(95)=316.13µs
     iterations.....................: 103647  3454.770329/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

@ardatan ardatan marked this pull request as draft December 27, 2024 10:17
@ardatan ardatan force-pushed the all-tests branch 2 times, most recently from 92c95c8 to 23fb6bd Compare December 27, 2024 11:56
@ardatan ardatan changed the title Run tests with all Node frameworks & fix URL Run tests with all Node frameworks & some fixes on URL and ReadableStream.cancel Dec 27, 2024
@ardatan ardatan force-pushed the all-tests branch 3 times, most recently from 763fdcd to e60dc5d Compare December 27, 2024 12:13
@ardatan ardatan marked this pull request as ready for review December 27, 2024 12:13
@ardatan ardatan force-pushed the all-tests branch 8 times, most recently from f3a0bdc to d2f2cdc Compare December 27, 2024 12:55
@ardatan ardatan requested review from Urigo and n1ru4l December 27, 2024 13:41
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.

2 participants