-
Notifications
You must be signed in to change notification settings - Fork 23
fix(@graphql-mesh/transport-http): infinite loop on subscriptions without kind #1385
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(@graphql-mesh/transport-http): infinite loop on subscriptions without kind #1385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an infinite loop bug in the HTTP transport when handling GraphQL subscriptions. The issue occurred when the subscriptions
option was passed to the subscription transport builder, causing recursive instantiation.
- Removes the
subscriptions
attribute from options passed to subscription transport - Adds explanatory comments for the fix
- Prevents infinite recursion in HTTP transport subscription handling
options: { | ||
...payload.transportEntry.options, | ||
...payload.transportEntry.options?.subscriptions?.options, | ||
// Make sure to remove subscription option to avoid infinite loop. | ||
// This option doesn't make sense here but is present in `transportEntry.options` | ||
subscriptions: undefined, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Instead of setting subscriptions: undefined
, consider using object destructuring with rest operator to exclude the subscriptions property entirely. This would be more explicit: const { subscriptions, ...optionsWithoutSubscriptions } = payload.transportEntry.options; options: { ...optionsWithoutSubscriptions, ...payload.transportEntry.options?.subscriptions?.options }
options: { | |
...payload.transportEntry.options, | |
...payload.transportEntry.options?.subscriptions?.options, | |
// Make sure to remove subscription option to avoid infinite loop. | |
// This option doesn't make sense here but is present in `transportEntry.options` | |
subscriptions: undefined, | |
}, | |
options: (() => { | |
const { subscriptions, ...optionsWithoutSubscriptions } = payload.transportEntry.options ?? {}; | |
return { | |
...optionsWithoutSubscriptions, | |
...payload.transportEntry.options?.subscriptions?.options, | |
}; | |
})(), |
Copilot uses AI. Check for mistakes.
🚀 Snapshot Release (
|
Package | Version | Info |
---|---|---|
@graphql-mesh/fusion-runtime |
0.11.20-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway |
1.16.3-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-json |
0.0.7-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-pino |
1.0.3-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-winston |
1.0.4-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/nestjs |
1.0.22-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-aws-sigv4 |
1.0.18-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-deduplicate-request |
1.0.4-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/hmac-upstream-signature |
1.2.31-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-jwt-auth |
1.5.8-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-opentelemetry |
1.3.66-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-prometheus |
1.3.54-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-runtime |
1.10.3-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-common |
0.7.38-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http |
0.7.2-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http-callback |
0.7.2-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-ws |
1.1.2-alpha-08c27dc7bbdeb05b6399827ceb8ef259cd41aa3a |
npm ↗︎ unpkg ↗︎ |
🚀 Snapshot Release (Node Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared
|
🚀 Snapshot Release (Bun Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but should we also cover this usecase in our tests?
Yeah I will try to add a unit test |
3af9992
to
a6c5fed
Compare
HTTP transport loading is causing infinite loop when
subscriptions
option is provided with thehttp
kind (or ifkind
is not specified since it defaults tohttp
).This is caused by the fact we pass all parent transport entry options to the subscription specific transport. In the case of the
http
transport, it was causing an infinite loop since a new subscriptions specific transport was instanciated every time becausesubscriptions
option was always present.This PR fixes this by explicitly removing the
subscriptions
attribute from the options passed to the subscription transport builder.Related to #1379