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

Internal context missing when there's no connection_init frame #451

Closed
leebenson opened this issue Mar 24, 2021 · 4 comments
Closed

Internal context missing when there's no connection_init frame #451

leebenson opened this issue Mar 24, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@leebenson
Copy link
Contributor

leebenson commented Mar 24, 2021

After working on vectordotdev/vector#6871, I discovered an error in our Vector API client where a start operation was sent before a connection_init message.

A side-effect to this is that context provided in graphql_subscription_with_data() isn't added, because there's no initial payload to merge with. This resulted in panics when using Context.data_unchecked.

It would be useful for 'internal' context to be injected without relying on a connection_init message from the client so this doesn't need to be a checked operation.

If it is provided, then this would be merged with the initial payload as normal.

Does that seem reasonable? Or would you recommend simply using checked operations at all times?

@leebenson leebenson added the bug Something isn't working label Mar 24, 2021
@sunli829
Copy link
Collaborator

I checked the protocol specification of graphql-ws, connection_init message must be sent first by the client during the handshake phase, so when the handshake phase is not completed, the client start a subscription should get an error.

What do you think?

@leebenson
Copy link
Contributor Author

Agreed. If connection_init is missing, I think we should probably avoid starting the subscription, rather than allowing it to be an in 'incomplete' state 👍🏻

@sunli829
Copy link
Collaborator

Now, if the client sends start before connection_init, the connection will be immediately disconnected and return 1011 error.

Released in v2.6.5

@leebenson
Copy link
Contributor Author

Awesome, thanks @sunli829 ! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants