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

fix: context properties are shared between batched requests #3491

Merged
merged 7 commits into from
Nov 14, 2024

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented Nov 13, 2024

While investigating an issue for usage reporting for batched requests, I realized that context assignments for one batch execution to one context are also shared/assigned to the other batch contexts.

Object.create creates a new object using the existing object as the prototype so the assignments seem to be inherited. 🫤

This bug was introduced in #3270

This bug originates from @whatwg-node/server See ardatan/whatwg-node#1797 and ardatan/whatwg-node#1796).

Copy link

changeset-bot bot commented Nov 13, 2024

🦋 Changeset detected

Latest commit: bcee059

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 24 packages
Name Type
graphql-yoga Patch
@graphql-yoga/nestjs Patch
@graphql-yoga/render-graphiql Patch
@graphql-yoga/plugin-apollo-inline-trace Patch
@graphql-yoga/apollo-managed-federation Patch
@graphql-yoga/plugin-apollo-usage-report Patch
@graphql-yoga/plugin-apq Patch
@graphql-yoga/plugin-csrf-prevention Patch
@graphql-yoga/plugin-defer-stream Patch
@graphql-yoga/plugin-disable-introspection Patch
@graphql-yoga/plugin-graphql-sse Patch
@graphql-yoga/plugin-jwt Patch
@graphql-yoga/plugin-persisted-operations Patch
@graphql-yoga/plugin-prometheus Patch
@graphql-yoga/plugin-response-cache Patch
@graphql-yoga/plugin-sofa Patch
apollo-federation-gateway-with-yoga Patch
apollo-subgraph-with-yoga Patch
graphql-lambda Patch
cloudflare-advanced Patch
cloudflare Patch
nextjs-app Patch
hello-world-benchmark Patch
@graphql-yoga/nestjs-federation Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor

github-actions bot commented Nov 13, 2024

💻 Website Preview

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

Copy link
Contributor

github-actions bot commented Nov 13, 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% ✓ 519280      ✗ 0     
     data_received................................: 2.1 GB  14 MB/s
     data_sent....................................: 104 MB  696 kB/s
     http_req_blocked.............................: avg=1.49µs   min=1µs      med=1.31µs   max=281.97µs p(90)=1.99µs   p(95)=2.19µs  
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=139.94µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=364.02µs min=222.17µs med=327.18µs max=19.51ms  p(90)=462.36µs p(95)=483.03µs
       { expected_response:true }.................: avg=364.02µs min=222.17µs med=327.18µs max=19.51ms  p(90)=462.36µs p(95)=483.03µs
     ✓ { mode:graphql-jit }.......................: avg=294.68µs min=222.17µs med=270.83µs max=19.51ms  p(90)=303.77µs p(95)=322.52µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=493.06µs min=402.89µs med=463.34µs max=9.65ms   p(90)=501.51µs p(95)=532.95µs
     ✓ { mode:graphql-response-cache }............: avg=344.33µs min=272.06µs med=324.7µs  max=7.75ms   p(90)=354.8µs  p(95)=366.46µs
     ✓ { mode:graphql }...........................: avg=372.71µs min=274.86µs med=336µs    max=13.68ms  p(90)=394.59µs p(95)=448.98µs
     ✓ { mode:uws }...............................: avg=350.15µs min=275.26µs med=327.55µs max=9.51ms   p(90)=360.61µs p(95)=378.68µs
     http_req_failed..............................: 0.00%   ✓ 0           ✗ 259640
     http_req_receiving...........................: avg=33.7µs   min=15.97µs  med=33.57µs  max=2.28ms   p(90)=39.39µs  p(95)=41.57µs 
     http_req_sending.............................: avg=8.58µs   min=6.06µs   med=7.53µs   max=3.28ms   p(90)=11.06µs  p(95)=12.12µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=321.73µs min=186.33µs med=285.85µs max=19.37ms  p(90)=419.51µs p(95)=437.67µs
     http_reqs....................................: 259640  1730.911286/s
     iteration_duration...........................: avg=572.94µs min=389.29µs med=533.57µs max=20.1ms   p(90)=674.32µs p(95)=699.67µs
     iterations...................................: 259640  1730.911286/s
     vus..........................................: 1       min=1         max=1   
     vus_max......................................: 2       min=2         max=2   

Copy link
Contributor

github-actions bot commented Nov 13, 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:

@n1ru4l n1ru4l force-pushed the fix-batching-context-identity branch from 6517f1a to 97fdef0 Compare November 14, 2024 10:06
@n1ru4l n1ru4l changed the title test: context properties are shared between fix: context properties are shared between batched requests Nov 14, 2024
@n1ru4l n1ru4l force-pushed the fix-batching-context-identity branch from 4198466 to b211d06 Compare November 14, 2024 10:12
@n1ru4l n1ru4l force-pushed the fix-batching-context-identity branch from 4142059 to 74d13c9 Compare November 14, 2024 10:20
@n1ru4l n1ru4l merged commit 7a413bc into main Nov 14, 2024
35 checks passed
@n1ru4l n1ru4l deleted the fix-batching-context-identity branch November 14, 2024 10:42
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