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

feat(node-fetch): improve file access #2009

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

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Jan 24, 2025

When fetch('file:///...') is used to read files;

  • 404 is returned if the file is missing
  • 403 is returned if the file is not accessible

Copy link
Contributor

github-actions bot commented Jan 24, 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/node-fetch 0.7.8-alpha-20250124215951-ab82e441881dc42c7b236e47e5cbd62a5e2a359d npm ↗︎ unpkg ↗︎

Copy link
Contributor

github-actions bot commented Jan 24, 2025

@benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 215670      ✗ 0     
     data_received..................: 22 MB   723 kB/s
     data_sent......................: 8.6 MB  288 kB/s
     http_req_blocked...............: avg=1.48µs   min=922ns    med=1.25µs   max=218.85µs p(90)=1.98µs   p(95)=2.15µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=139.32µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=214.01µs min=158.55µs med=201.75µs max=63.99ms  p(90)=227.77µs p(95)=237.53µs
       { expected_response:true }...: avg=214.01µs min=158.55µs med=201.75µs max=63.99ms  p(90)=227.77µs p(95)=237.53µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 107835
     http_req_receiving.............: avg=26.05µs  min=13.9µs   med=24.44µs  max=3.09ms   p(90)=31.73µs  p(95)=34.26µs 
     http_req_sending...............: avg=6.69µs   min=4.13µs   med=5.98µs   max=405.82µs p(90)=8.42µs   p(95)=9.37µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=181.26µs min=131.11µs med=169.49µs max=63.91ms  p(90)=191.84µs p(95)=200.66µs
     http_reqs......................: 107835  3594.303278/s
     iteration_duration.............: avg=273.5µs  min=207.68µs med=260.05µs max=64.14ms  p(90)=290.16µs p(95)=302.77µs
     iterations.....................: 107835  3594.303278/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Jan 24, 2025

@benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 285666      ✗ 0     
     data_received..................: 28 MB   938 kB/s
     data_sent......................: 11 MB   381 kB/s
     http_req_blocked...............: avg=1.5µs    min=882ns    med=1.26µs   max=274.77µs p(90)=1.99µs   p(95)=2.22µs  
     http_req_connecting............: avg=0ns      min=0s       med=0s       max=126.06µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=144.88µs min=96.36µs  med=139.01µs max=6.19ms   p(90)=163.83µs p(95)=172.79µs
       { expected_response:true }...: avg=144.88µs min=96.36µs  med=139.01µs max=6.19ms   p(90)=163.83µs p(95)=172.79µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 142833
     http_req_receiving.............: avg=25.38µs  min=12.23µs  med=23.43µs  max=3.1ms    p(90)=31.79µs  p(95)=35.74µs 
     http_req_sending...............: avg=6.72µs   min=4.09µs   med=5.95µs   max=290.16µs p(90)=8.5µs    p(95)=10.12µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=112.78µs min=70.25µs  med=106.66µs max=6.12ms   p(90)=127.88µs p(95)=135.72µs
     http_reqs......................: 142833  4760.922449/s
     iteration_duration.............: avg=205.3µs  min=142.53µs med=198.7µs  max=6.44ms   p(90)=227.2µs  p(95)=239.14µs
     iterations.....................: 142833  4760.922449/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Jan 24, 2025

@benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=139.294243 min=13     med=137     max=190     p(90)=162     p(95)=167    
     data_received..................: 21 MB  703 kB/s
     data_sent......................: 14 MB  451 kB/s
     http_req_blocked...............: avg=2.04µs     min=661ns  med=1.23µs  max=4.61ms  p(90)=2µs     p(95)=2.27µs 
     http_req_connecting............: avg=194ns      min=0s     med=0s      max=2.43ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=21.69ms    min=2.43ms med=21.11ms max=1.14s   p(90)=27.25ms p(95)=28.84ms
       { expected_response:true }...: avg=21.69ms    min=2.43ms med=21.11ms max=1.14s   p(90)=27.25ms p(95)=28.84ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 137840
     http_req_receiving.............: avg=32.39µs    min=8.86µs med=22.68µs max=23.61ms p(90)=36.6µs  p(95)=43.52µs
     http_req_sending...............: avg=9.91µs     min=3.26µs med=5.76µs  max=7.96ms  p(90)=9.42µs  p(95)=13.28µs
     http_req_tls_handshaking.......: avg=0s         min=0s     med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=21.65ms    min=2.39ms med=21.07ms max=1.14s   p(90)=27.21ms p(95)=28.78ms
     http_reqs......................: 137840 4593.929543/s
     iteration_duration.............: avg=43.49ms    min=9.81ms med=41.92ms max=1.16s   p(90)=47.09ms p(95)=52.92ms
     iterations.....................: 68909  2296.598164/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Jan 24, 2025

@benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=139.623058 min=18      med=141     max=198     p(90)=160     p(95)=166    
     data_received..................: 19 MB  620 kB/s
     data_sent......................: 12 MB  402 kB/s
     http_req_blocked...............: avg=3.15µs     min=601ns   med=1.47µs  max=7.3ms   p(90)=2.15µs  p(95)=2.57µs 
     http_req_connecting............: avg=1.14µs     min=0s      med=0s      max=3.37ms  p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=24.59ms    min=4.42ms  med=23.9ms  max=1.27s   p(90)=30.84ms p(95)=32.81ms
       { expected_response:true }...: avg=24.59ms    min=4.42ms  med=23.9ms  max=1.27s   p(90)=30.84ms p(95)=32.81ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 121592
     http_req_receiving.............: avg=39.19µs    min=11.26µs med=29.43µs max=18.36ms p(90)=43.83µs p(95)=53.1µs 
     http_req_sending...............: avg=12.89µs    min=4.13µs  med=7.95µs  max=16.94ms p(90)=11.63µs p(95)=16.56µs
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=24.54ms    min=4.36ms  med=23.86ms max=1.27s   p(90)=30.79ms p(95)=32.73ms
     http_reqs......................: 121592 4052.182648/s
     iteration_duration.............: avg=49.31ms    min=13.84ms med=47.48ms max=1.29s   p(90)=53.41ms p(95)=59.43ms
     iterations.....................: 60768  2025.158194/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Jan 24, 2025

@benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 208668     ✗ 0     
     data_received..................: 21 MB   699 kB/s
     data_sent......................: 8.3 MB  278 kB/s
     http_req_blocked...............: avg=1.42µs   min=851ns    med=1.21µs   max=184.29µs p(90)=1.91µs   p(95)=2.09µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=131.39µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=224.77µs min=168.44µs med=214.16µs max=37.13ms  p(90)=241.71µs p(95)=252.81µs
       { expected_response:true }...: avg=224.77µs min=168.44µs med=214.16µs max=37.13ms  p(90)=241.71µs p(95)=252.81µs
     http_req_failed................: 0.00%   ✓ 0          ✗ 104334
     http_req_receiving.............: avg=25.6µs   min=14.02µs  med=23.85µs  max=2.51ms   p(90)=30.96µs  p(95)=33.73µs 
     http_req_sending...............: avg=6.49µs   min=4.09µs   med=5.66µs   max=320.33µs p(90)=8.28µs   p(95)=9.04µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=192.66µs min=146.46µs med=182.18µs max=37.06ms  p(90)=206.92µs p(95)=217.29µs
     http_reqs......................: 104334  3477.68312/s
     iteration_duration.............: avg=283.04µs min=211.08µs med=271.62µs max=37.27ms  p(90)=302.79µs p(95)=316.41µs
     iterations.....................: 104334  3477.68312/s
     vus............................: 1       min=1        max=1   
     vus_max........................: 1       min=1        max=1   

@ardatan
Copy link
Owner Author

ardatan commented Jan 29, 2025

What do you think @coderabbitai

Copy link

coderabbitai bot commented Jan 29, 2025

@ardatan The changes look great! I particularly like:

  1. Proper error handling with specific status codes (404 for missing files, 403 for access denied)
  2. Addition of useful response headers (content-type, last-modified)
  3. Making the file handling properly async
  4. Comprehensive test coverage for all scenarios

The implementation is clean and achieves the PR objectives perfectly. I recommend merging these changes.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch PR details and diff
gh pr view 2009 --json files
gh pr diff 2009

# Search for relevant test files
fd -e test.ts -e spec.ts node-fetch

Length of output: 4236

✅ Actions performed

Comments resolved.

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