Skip to content

Conversation

ardatan
Copy link
Owner

@ardatan ardatan commented Mar 16, 2025

  • Name functions in iterateAsync for more readable traces
  • fakePromise accepts MaybePromise as an input
  • Use Array.fromAsync when possible to collect values

Related graphql-hive/gateway#862

Copy link
Contributor

coderabbitai bot commented Mar 16, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Enhanced asynchronous data collection, enabling more efficient handling of data streams.
  • Refactor

    • Improved promise management for clearer traceability, better resolution of nested asynchronous operations, and streamlined processing logic.

Walkthrough

The pull request introduces updates in both the @whatwg-node/promise-helpers and @whatwg-node/node-fetch packages. In the promise-helpers package, the fakePromise function signature is updated to return a more accurately awaited type, and inline callbacks in iterateAsync are replaced with named functions for better trace readability. In the node-fetch package, the PonyfillBody class now leverages the handleMaybePromise function to streamline asynchronous handling, and a changeset entry promotes using Array.fromAsync for asynchronous collections.

Changes

File(s) Change Summary
.changeset/loose-cities-grin.md
packages/promise-helpers/src/index.ts
Updates to @whatwg-node/promise-helpers: the fakePromise signature is modified to return Promise<Awaited<T>>, and the iterateAsync method now uses named functions (handleCallback and handleCallbackResult) for improved trace readability.
.changeset/quiet-corners-tickle.md
packages/node-fetch/src/Body.ts
Patch for @whatwg-node/node-fetch: Introduces asynchronous collection with Array.fromAsync in the changeset and integrates handleMaybePromise in the PonyfillBody class to streamline promise handling in both async iterable and readable stream scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PonyfillBody
    participant handleMaybePromise
    Client->>PonyfillBody: Request blob/buffer
    PonyfillBody->>handleMaybePromise: Convert stream/iterable to array
    handleMaybePromise-->>PonyfillBody: Return chunk array
    PonyfillBody-->>Client: Return blob/buffer
Loading
sequenceDiagram
    participant Caller
    participant iterateAsync
    participant handleCallback
    participant handleCallbackResult
    Caller->>iterateAsync: Invoke iterateAsync with callback
    iterateAsync->>handleCallback: Execute callback
    handleCallback-->>iterateAsync: Return callback result
    iterateAsync->>handleCallbackResult: Process result
    handleCallbackResult-->>iterateAsync: Return processed value
    iterateAsync-->>Caller: Return final promise result
Loading

Possibly related PRs

  • Introduce Promise Helpers #2102: Focuses on modifications to the fakePromise function in the @whatwg-node/promise-helpers package, directly relating to adjustments made in this PR.
  • Upcoming Release Changes #2104: Involves changes linked to the fakePromise function and dependency integration, matching the updates in this PR.
  • Upcoming Release Changes #2108: Addresses issues in the iterateAsync function, complementing the refactoring changes introduced in this pull request.

Suggested reviewers

  • dotansimha
  • EmrysMyrddin

Poem

I'm a rabbit hopping through lines of code,
Named functions now in my humble abode.
Async promises neatly aligned in a row,
With streams and callbacks ready to go.
Code carrots await, and off we hop! 🥕🐰

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2646e98 and 6b5d97c.

📒 Files selected for processing (4)
  • .changeset/loose-cities-grin.md (1 hunks)
  • .changeset/quiet-corners-tickle.md (1 hunks)
  • packages/node-fetch/src/Body.ts (4 hunks)
  • packages/promise-helpers/src/index.ts (2 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ardatan ardatan force-pushed the small-improvements branch from 8064d17 to 3c5910f Compare March 16, 2025 01:31
Copy link
Contributor

github-actions bot commented Mar 16, 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.13-alpha-20250316014213-6b5d97cf20e1c790fba9f3c11c0cd0f6a3f67fa0 npm ↗︎ unpkg ↗︎
@whatwg-node/promise-helpers 1.2.5-alpha-20250316014213-6b5d97cf20e1c790fba9f3c11c0cd0f6a3f67fa0 npm ↗︎ unpkg ↗︎

Copy link
Contributor

github-actions bot commented Mar 16, 2025

@benchmarks/node-fetch results (consumeBody)

   ✓ active_handles.................: avg=140.250188 min=12      med=139     max=184     p(90)=159     p(95)=163    
     data_received..................: 20 MB  664 kB/s
     data_sent......................: 13 MB  425 kB/s
     http_req_blocked...............: avg=2.18µs     min=601ns   med=1.38µs  max=3.15ms  p(90)=2.11µs  p(95)=2.43µs 
     http_req_connecting............: avg=251ns      min=0s      med=0s      max=3.1ms   p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=22.98ms    min=3.43ms  med=22.39ms max=1.05s   p(90)=28.82ms p(95)=30.42ms
       { expected_response:true }...: avg=22.98ms    min=3.43ms  med=22.39ms max=1.05s   p(90)=28.82ms p(95)=30.42ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 130133
     http_req_receiving.............: avg=36.54µs    min=9.91µs  med=24.9µs  max=17.4ms  p(90)=39.36µs p(95)=46.06µs
     http_req_sending...............: avg=11.02µs    min=3.53µs  med=6.55µs  max=15.86ms p(90)=10.27µs p(95)=14.31µs
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=22.93ms    min=3.39ms  med=22.35ms max=1.05s   p(90)=28.78ms p(95)=30.36ms
     http_reqs......................: 130133 4337.197402/s
     iteration_duration.............: avg=46.07ms    min=10.47ms med=44.64ms max=1.07s   p(90)=49.48ms p(95)=55.72ms
     iterations.....................: 65039  2167.682155/s
     vus............................: 100    min=100       max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Mar 16, 2025

@benchmarks/server results (native)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 230324      ✗ 0     
     data_received..................: 23 MB   772 kB/s
     data_sent......................: 9.2 MB  307 kB/s
     http_req_blocked...............: avg=1.39µs   min=881ns    med=1.18µs   max=172.59µs p(90)=1.87µs   p(95)=2.03µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=122.23µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=197.89µs min=141.58µs med=188.22µs max=8.45ms   p(90)=213.58µs p(95)=223.02µs
       { expected_response:true }...: avg=197.89µs min=141.58µs med=188.22µs max=8.45ms   p(90)=213.58µs p(95)=223.02µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 115162
     http_req_receiving.............: avg=25.61µs  min=13.71µs  med=23.93µs  max=2.93ms   p(90)=30.65µs  p(95)=33.52µs 
     http_req_sending...............: avg=6.45µs   min=4.05µs   med=5.59µs   max=281.44µs p(90)=8.25µs   p(95)=8.95µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=165.82µs min=118.17µs med=156.4µs  max=8.39ms   p(90)=178.99µs p(95)=187.59µs
     http_reqs......................: 115162  3838.613704/s
     iteration_duration.............: avg=255.93µs min=183.39µs med=245.36µs max=8.55ms   p(90)=273.81µs p(95)=285.47µs
     iterations.....................: 115162  3838.613704/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Mar 16, 2025

@benchmarks/node-fetch results (noConsumeBody)

   ✓ active_handles.................: avg=140.301885 min=30      med=140     max=183     p(90)=161     p(95)=165    
     data_received..................: 21 MB  711 kB/s
     data_sent......................: 14 MB  461 kB/s
     http_req_blocked...............: avg=3.9µs      min=631ns   med=1.34µs  max=9.02ms  p(90)=2.02µs  p(95)=2.26µs 
     http_req_connecting............: avg=2.15µs     min=0s      med=0s      max=8.9ms   p(90)=0s      p(95)=0s     
     http_req_duration..............: avg=21.43ms    min=4.86ms  med=20.86ms max=1.03s   p(90)=27.08ms p(95)=28.48ms
       { expected_response:true }...: avg=21.43ms    min=4.86ms  med=20.86ms max=1.03s   p(90)=27.08ms p(95)=28.48ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 139470
     http_req_receiving.............: avg=34.37µs    min=9.47µs  med=22.99µs max=24.97ms p(90)=37.57µs p(95)=44.1µs 
     http_req_sending...............: avg=11.02µs    min=3.39µs  med=6.27µs  max=21.2ms  p(90)=9.72µs  p(95)=13.62µs
     http_req_tls_handshaking.......: avg=0s         min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...............: avg=21.39ms    min=4.7ms   med=20.82ms max=1.03s   p(90)=27.04ms p(95)=28.4ms 
     http_reqs......................: 139470 4648.53071/s
     iteration_duration.............: avg=42.98ms    min=18.09ms med=41.51ms max=1.05s   p(90)=45.85ms p(95)=52.74ms
     iterations.....................: 69722  2323.832065/s
     vus............................: 0      min=0         max=100 
     vus_max........................: 100    min=100       max=100 

Copy link
Contributor

github-actions bot commented Mar 16, 2025

@benchmarks/server results (ponyfill)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 292314      ✗ 0     
     data_received..................: 29 MB   960 kB/s
     data_sent......................: 12 MB   390 kB/s
     http_req_blocked...............: avg=1.39µs   min=872ns   med=1.17µs   max=293.58µs p(90)=1.88µs   p(95)=2.06µs  
     http_req_connecting............: avg=1ns      min=0s      med=0s       max=148.8µs  p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=142µs    min=97.41µs med=137.14µs max=5.92ms   p(90)=159.37µs p(95)=166.72µs
       { expected_response:true }...: avg=142µs    min=97.41µs med=137.14µs max=5.92ms   p(90)=159.37µs p(95)=166.72µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 146157
     http_req_receiving.............: avg=24.41µs  min=12.35µs med=22.85µs  max=2.68ms   p(90)=30.16µs  p(95)=32.89µs 
     http_req_sending...............: avg=6.33µs   min=4.07µs  med=5.49µs   max=355.22µs p(90)=8.14µs   p(95)=8.9µs   
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=111.25µs min=70.56µs med=105.94µs max=5.79ms   p(90)=125.2µs  p(95)=131.15µs
     http_reqs......................: 146157  4871.720239/s
     iteration_duration.............: avg=200.7µs  min=143.9µs med=195.25µs max=6.25ms   p(90)=220.89µs p(95)=230.2µs 
     iterations.....................: 146157  4871.720239/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

Copy link
Contributor

github-actions bot commented Mar 16, 2025

@benchmarks/server results (undici)

     ✓ no-errors
     ✓ expected-result

   ✓ checks.........................: 100.00% ✓ 213080      ✗ 0     
     data_received..................: 21 MB   714 kB/s
     data_sent......................: 8.5 MB  284 kB/s
     http_req_blocked...............: avg=1.48µs   min=872ns    med=1.22µs   max=2.16ms   p(90)=1.97µs   p(95)=2.17µs  
     http_req_connecting............: avg=1ns      min=0s       med=0s       max=120.37µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=216.95µs min=156.85µs med=204.5µs  max=51.73ms  p(90)=230.73µs p(95)=240.75µs
       { expected_response:true }...: avg=216.95µs min=156.85µs med=204.5µs  max=51.73ms  p(90)=230.73µs p(95)=240.75µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 106540
     http_req_receiving.............: avg=26.28µs  min=14.19µs  med=24.55µs  max=2.71ms   p(90)=31.74µs  p(95)=34.47µs 
     http_req_sending...............: avg=6.66µs   min=4.08µs   med=6µs      max=381.34µs p(90)=8.44µs   p(95)=9.43µs  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=184µs    min=128.82µs med=172µs    max=51.65ms  p(90)=195.08µs p(95)=204.01µs
     http_reqs......................: 106540  3551.209968/s
     iteration_duration.............: avg=276.88µs min=202.29µs med=263.37µs max=51.9ms   p(90)=293.49µs p(95)=305.81µs
     iterations.....................: 106540  3551.209968/s
     vus............................: 1       min=1         max=1   
     vus_max........................: 1       min=1         max=1   

@ardatan ardatan marked this pull request as ready for review March 17, 2025 12:54
@ardatan ardatan merged commit a45e929 into master Mar 17, 2025
25 checks passed
@ardatan ardatan deleted the small-improvements branch May 12, 2025 16:19
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