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.

Sign up for GitHub

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

Jump to bottom

enhance(node-fetch): use undici when available #1997

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

enhance(node-fetch): use undici when available #1997

wants to merge 3 commits into from

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Jan 22, 2025

No description provided.

@ardatan ardatan force-pushed the use-undici-if-available branch from 14266b4 to 8725fae Compare January 22, 2025 13:48
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

🚀 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.4-alpha-20250122230954-3105b2fac861fe650311d5dc0f4a88470234b0b5 npm ↗︎ unpkg ↗︎
@whatwg-node/node-fetch 0.7.8-alpha-20250122230954-3105b2fac861fe650311d5dc0f4a88470234b0b5 npm ↗︎ unpkg ↗︎

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

✅ @benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=102.425213 min=54     med=103     max=103     p(90)=103     p(95)=103    
     data_received..................: 14 MB  457 kB/s
     data_sent......................: 8.9 MB 296 kB/s
     http_req_blocked...............: avg=4.29µs     min=591ns  med=1.66µs  max=7.08ms  p(90)=2.16µs  p(95)=2.53µs 
     http_req_connecting............: avg=1.95µs     min=0s     med=0s      max=5.03ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=33.38ms    min=4.71ms med=34.76ms max=1.21s   p(90)=53.85ms p(95)=62.88ms
       { expected_response:true }...: avg=33.38ms    min=4.71ms med=34.76ms max=1.21s   p(90)=53.85ms p(95)=62.88ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 89614
     http_req_receiving.............: avg=30.62µs    min=10.3µs med=25.78µs max=11.54ms p(90)=41.15µs p(95)=47.29µs
     http_req_sending...............: avg=11.32µs    min=3.73µs med=8.56µs  max=5.43ms  p(90)=12.94µs p(95)=15.98µs
     http_req_tls_handshaking.......: avg=0s         min=0s     med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=33.34ms    min=4.12ms med=34.7ms  max=1.21s   p(90)=53.8ms  p(95)=62.84ms
     http_reqs......................: 89614  2986.683368/s
     iteration_duration.............: avg=66.88ms    min=36.3ms med=64.14ms max=1.23s   p(90)=77.78ms p(95)=81.22ms
     iterations.....................: 44794  1492.908416/s
     vus............................: 100    min=100       max=100
     vus_max........................: 100    min=100       max=100

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

✅ @benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=102.458074 min=43      med=103     max=103     p(90)=103     p(95)=103    
     data_received..................: 14 MB  473 kB/s
     data_sent......................: 9.1 MB 304 kB/s
     http_req_blocked...............: avg=5.7µs      min=621ns   med=1.6µs   max=10.24ms p(90)=2.1µs   p(95)=2.44µs 
     http_req_connecting............: avg=3.54µs     min=0s      med=0s      max=6ms     p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=32.23ms    min=1.13ms  med=36.47ms max=1.04s   p(90)=59.15ms p(95)=62.61ms
       { expected_response:true }...: avg=32.23ms    min=1.13ms  med=36.47ms max=1.04s   p(90)=59.15ms p(95)=62.61ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 92813
     http_req_receiving.............: avg=34.49µs    min=8.8µs   med=24.09µs max=9.59ms  p(90)=40.47µs p(95)=50.11µs
     http_req_sending...............: avg=11.82µs    min=3.11µs  med=7.92µs  max=13.31ms p(90)=11.83µs p(95)=16.35µs
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=32.18ms    min=1.09ms  med=36.34ms max=1.04s   p(90)=59.11ms p(95)=62.54ms
     http_reqs......................: 92813  3093.497731/s
     iteration_duration.............: avg=64.59ms    min=43.61ms med=62.19ms max=1.07s   p(90)=71.72ms p(95)=77.22ms
     iterations.....................: 46403  1546.632209/s
     vus............................: 100    min=100       max=100
     vus_max........................: 100    min=100       max=100

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

✅ @benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 218004     ✗ 0     
     data_received..................: 22 MB   730 kB/s
     data_sent......................: 8.7 MB  291 kB/s
     http_req_blocked...............: avg=1.46µs   min=921ns    med=1.23µs   max=284.3µs  p(90)=1.94µs   p(95)=2.13µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=122.1µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=211.84µs min=155.89µs med=200.23µs max=48.1ms   p(90)=226.27µs p(95)=235.53µs
       { expected_response:true }...: avg=211.84µs min=155.89µs med=200.23µs max=48.1ms   p(90)=226.27µs p(95)=235.53µs
     http_req_failed................: 0.00%   ✓ 0          ✗ 109002
     http_req_receiving.............: avg=25.74µs  min=13.33µs  med=24.06µs  max=2.9ms    p(90)=30.96µs  p(95)=33.6µs  
     http_req_sending...............: avg=6.51µs   min=4.14µs   med=5.73µs   max=327.73µs p(90)=8.23µs   p(95)=8.99µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=179.59µs min=127.14µs med=168.39µs max=48.02ms  p(90)=191.19µs p(95)=199.64µs
     http_reqs......................: 109002  3633.27291/s
     iteration_duration.............: avg=270.61µs min=199.43µs med=258.1µs  max=48.21ms  p(90)=287.63µs p(95)=299.15µs
     iterations.....................: 109002  3633.27291/s
     vus............................: 1       min=1        max=1   
     vus_max........................: 1       min=1        max=1

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

✅ @benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 204732      ✗ 0     
     data_received..................: 21 MB   686 kB/s
     data_sent......................: 8.2 MB  273 kB/s
     http_req_blocked...............: avg=1.45µs   min=872ns    med=1.21µs   max=201.06µs p(90)=1.96µs   p(95)=2.15µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=137.48µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=229.51µs min=170.62µs med=218.69µs max=12.47ms  p(90)=246.25µs p(95)=257.66µs
       { expected_response:true }...: avg=229.51µs min=170.62µs med=218.69µs max=12.47ms  p(90)=246.25µs p(95)=257.66µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 102366
     http_req_receiving.............: avg=26.11µs  min=13.87µs  med=24.6µs   max=2.68ms   p(90)=31.52µs  p(95)=34.4µs  
     http_req_sending...............: avg=6.62µs   min=4.06µs   med=5.81µs   max=2.03ms   p(90)=8.4µs    p(95)=9.36µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=196.77µs min=144.02µs med=186.22µs max=12.39ms  p(90)=210.85µs p(95)=221.06µs
     http_reqs......................: 102366  3412.027665/s
     iteration_duration.............: avg=288.48µs min=221.16µs med=276.77µs max=12.61ms  p(90)=307.97µs p(95)=322.03µs
     iterations.....................: 102366  3412.027665/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

✅ @benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 304590      ✗ 0     
     data_received..................: 30 MB   1.0 MB/s
     data_sent......................: 12 MB   406 kB/s
     http_req_blocked...............: avg=1.34µs   min=851ns    med=1.15µs   max=200.13µs p(90)=1.82µs   p(95)=1.98µs  
     http_req_connecting............: avg=0ns      min=0s       med=0s       max=120.6µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=136.06µs min=93.51µs  med=131.8µs  max=5.79ms   p(90)=154.01µs p(95)=160.92µs
       { expected_response:true }...: avg=136.06µs min=93.51µs  med=131.8µs  max=5.79ms   p(90)=154.01µs p(95)=160.92µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 152295
     http_req_receiving.............: avg=24.02µs  min=11.93µs  med=22.51µs  max=2.81ms   p(90)=29.7µs   p(95)=32.4µs  
     http_req_sending...............: avg=6.05µs   min=3.9µs    med=5.18µs   max=270.54µs p(90)=7.94µs   p(95)=8.5µs   
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=105.99µs min=67.9µs   med=101.39µs max=5.62ms   p(90)=120.5µs  p(95)=126.3µs 
     http_reqs......................: 152295  5076.326122/s
     iteration_duration.............: avg=192.66µs min=137.95µs med=187.65µs max=6.42ms   p(90)=213.32µs p(95)=222.09µs
     iterations.....................: 152295  5076.326122/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1

@ardatan ardatan force-pushed the use-undici-if-available branch 4 times, most recently from 187117d to 99aa3b0 Compare January 22, 2025 15:04
@ardatan ardatan force-pushed the use-undici-if-available branch from 99aa3b0 to 7ab25b3 Compare January 22, 2025 15:12
@github-actions GitHub Actions
Copy link
Contributor

✅ @benchmarks/node-fetch results

   ✓ active_handles.................: avg=102.713476 min=6        med=103     max=103      p(90)=103     p(95)=103    
     data_received..................: 567 MB  2.4 MB/s
     data_sent......................: 369 MB  1.5 MB/s
     http_req_blocked...............: avg=1.84µs     min=501ns    med=1.28µs  max=10.76ms  p(90)=1.83µs  p(95)=2µs    
     http_req_connecting............: avg=250ns      min=0s       med=0s      max=7.98ms   p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=6.56ms     min=97.44µs  med=6.5ms   max=461.95ms p(90)=7.28ms  p(95)=7.62ms 
       { expected_response:true }...: avg=6.56ms     min=124.48µs med=6.53ms  max=57.53ms  p(90)=7.3ms   p(95)=7.63ms 
     http_req_failed................: 50.00%  ✓ 1810253      ✗ 1809856
     http_req_receiving.............: avg=23.32µs    min=6.26µs   med=17.16µs max=39.11ms  p(90)=28.75µs p(95)=34.03µs
     http_req_sending...............: avg=8.27µs     min=2.33µs   med=5.87µs  max=31.27ms  p(90)=8.93µs  p(95)=11.63µs
     http_req_tls_handshaking.......: avg=0s         min=0s       med=0s      max=0s       p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=6.53ms     min=64.57µs  med=6.48ms  max=461.76ms p(90)=7.25ms  p(95)=7.58ms 
     http_reqs......................: 3620109 15083.610892/s
     iteration_duration.............: avg=13.24ms    min=335.16µs med=13.1ms  max=476.68ms p(90)=14.34ms p(95)=15.7ms 
     iterations.....................: 1809853 7540.965872/s
     vus............................: 100     min=100        max=100  
     vus_max........................: 100     min=100        max=100

@ardatan
Copy link
Owner Author

ardatan commented Jan 29, 2025

What do you think @coderabbitai

@coderabbitai coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2025

@ardatan The changes look great! Adding undici support is a good enhancement that will improve performance when available. I particularly like:

  1. The prioritized fallback chain (libcurl → undici → built-in undici → node:http)
  2. The modular implementation with separate files for each fetch strategy
  3. The comprehensive test coverage including benchmarks for different implementations
  4. Proper error handling and graceful fallbacks

The implementation is clean and well-structured. I approve these changes.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch PR details
gh pr view 1997 --json files,commits,body

# Get the diff
gh pr diff 1997

# Search for relevant files
fd -e js -e ts node-fetch

Length of output: 58942

✏️ Learnings added 
Learnt from: ardatan
URL: https://github.com/ardatan/whatwg-node/pull/1997

Timestamp: 2025-01-29T19:45:35.637Z
Learning: The node-fetch package tries different fetch implementations in the following order: 1) node-libcurl 2) undici 3) built-in undici in Node 4) node:http as fallback

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Actions performed

Comments resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ardatan