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

chore: fastify example standard fetch #3539

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jasonkuhrt
Copy link
Collaborator

@jasonkuhrt jasonkuhrt commented Dec 11, 2024

Simplify the Fastify example by using standard fetch. Benefits:

  1. Tests check for the standard error type that is thrown in case of triggered abort controller.
  2. Fewer dependencies
  3. Avoid ipv6 bug

Following this change, I will update the other examples too.

Copy link

changeset-bot bot commented Dec 11, 2024

⚠️ No Changeset found

Latest commit: c99037f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 11, 2024

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

@jasonkuhrt jasonkuhrt requested review from n1ru4l and ardatan December 11, 2024 13:50
Copy link
Contributor

github-actions bot commented Dec 11, 2024

💻 Website Preview

The latest changes are available as preview in: https://81b5cacb.graphql-yoga.pages.dev

Copy link
Contributor

github-actions bot commented Dec 11, 2024

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}
     ✓ no_errors{mode:uws}
     ✓ expected_result{mode:uws}

     checks.......................................: 100.00% ✓ 506380      ✗ 0     
     data_received................................: 2.1 GB  14 MB/s
     data_sent....................................: 102 MB  678 kB/s
     http_req_blocked.............................: avg=1.57µs   min=972ns    med=1.36µs   max=282.35µs p(90)=2.08µs   p(95)=2.3µs   
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=144.71µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=371.28µs min=224.08µs med=338.01µs max=15.26ms  p(90)=479.04µs p(95)=502.36µs
       { expected_response:true }.................: avg=371.28µs min=224.08µs med=338.01µs max=15.26ms  p(90)=479.04µs p(95)=502.36µs
     ✓ { mode:graphql-jit }.......................: avg=297.46µs min=224.08µs med=279.57µs max=15.26ms  p(90)=311.77µs p(95)=329.18µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=505.58µs min=420.8µs  med=478.9µs  max=6.5ms    p(90)=521.25µs p(95)=564.56µs
     ✓ { mode:graphql-response-cache }............: avg=352.37µs min=270.23µs med=334.21µs max=6.47ms   p(90)=365.86µs p(95)=378.2µs 
     ✓ { mode:graphql }...........................: avg=380.12µs min=282.06µs med=349.67µs max=15.05ms  p(90)=416.5µs  p(95)=483.96µs
     ✓ { mode:uws }...............................: avg=357.49µs min=277.48µs med=337.75µs max=6.82ms   p(90)=375.06µs p(95)=399.18µs
     http_req_failed..............................: 0.00%   ✓ 0           ✗ 253190
     http_req_receiving...........................: avg=34.04µs  min=16.85µs  med=33.6µs   max=3.05ms   p(90)=40.27µs  p(95)=43.66µs 
     http_req_sending.............................: avg=8.92µs   min=5.96µs   med=7.92µs   max=465.56µs p(90)=11.37µs  p(95)=12.69µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=328.3µs  min=189.67µs med=296.17µs max=15.11ms  p(90)=435.66µs p(95)=457.26µs
     http_reqs....................................: 253190  1687.919312/s
     iteration_duration...........................: avg=587.41µs min=403.33µs med=549.96µs max=15.86ms  p(90)=697.16µs p(95)=725.37µs
     iterations...................................: 253190  1687.919312/s
     vus..........................................: 1       min=1         max=1   
     vus_max......................................: 2       min=2         max=2   

Copy link
Collaborator

@ardatan ardatan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is good to use @whatwg-node/fetch.
Thanks to catching this IPV6 bug here, we could fix the issue in whatwg-node/fetch which helped our other tools using @whatwg-node/fetch
Even if this is not a test for whatwg-node/fetch, it is still valuable to use it in our tests.

Also ipv6 is reproduced here during the lockfile upgrade, and fixed after bumping to the patch version;
https://github.com/dotansimha/graphql-yoga/actions/runs/12264473643/job/34218403800

@jasonkuhrt
Copy link
Collaborator Author

@ardatan Good to note that tradeoff!

@@ -24201,6 +24198,23 @@ snapshots:
'@swc/[email protected]':
optional: true

'@swc/[email protected]':
Copy link
Collaborator

@ardatan ardatan Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff looks weird 🤔

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