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

[WebSocketLink] inactivityTimeout causes a reconnect #430

Open
anlumo opened this issue Nov 17, 2023 · 5 comments
Open

[WebSocketLink] inactivityTimeout causes a reconnect #430

anlumo opened this issue Nov 17, 2023 · 5 comments

Comments

@anlumo
Copy link

anlumo commented Nov 17, 2023

When a WebSocketLink is closed due to the inactivityTimeout running out, it's automatically reconnected due to autoReconnect = true. This defeats the purpose of an inactivityTimeout.

@anlumo anlumo changed the title inactivityTimeout causes a reconnect [WebSocketLink] inactivityTimeout causes a reconnect Nov 17, 2023
@knaeckeKami
Copy link
Collaborator

Can you use the TransportWebsocketLink instead?

The architecture of TransportWebsocketLink is generally better, and the protocol that transport WebSocketLink implements is deprecated.

I imagine there will probably not a lot of maintenance work flow into WebSocketLink (unless someone steps up ;) )

@anlumo
Copy link
Author

anlumo commented Nov 17, 2023

Then please add the Deprecated annotation. I wasn't aware of that, and the documentation also doesn't say it.

@knaeckeKami
Copy link
Collaborator

knaeckeKami commented Nov 17, 2023

Well, there are two different protocols for graphql over Websockets.

graphql-transport-ws ( https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md )
and
graphql-ws
( https://github.com/apollographql/subscriptions-transport-ws/blob/master/PROTOCOL.md ).

To make matters worse, the javascript library implementing the graphql-transport-ws protocol is named graphql-ws ( https://www.npmjs.com/package/graphql-ws ) so there is a lot of potential for confusion.
The library that implemented the graphql-ws protocol was called subscriptions-transport-ws https://github.com/apollographql/subscriptions-transport-ws/tree/master

The graphql-ws protocol is deprecated. This is stated in the README.

But I would not completely deprecate the WebsocketLink class, there are still a lot of servers that only support this protocol.

However, if your server supports both, you should definitely use the graphql-transport-ws and TransportWebsocketLink class. Aside from the improved protocol, the dart implementation of the TransportWebsocketLink is better.

If you have suggestions on how the documentation around this can be improved, feel free to open issues or PRs.

@anlumo
Copy link
Author

anlumo commented Nov 18, 2023

If it's broken and not going to be fixed, isn't that the definition of deprecation? If you don't want people to encounter warnings when using that class, maybe just include what you told me here in the documentation of that class.

TransportWebsocketLink works fine for me, btw. It also doesn't have the bug that the WebSocket protocol name isn't set automatically on connection (and so the server rejects the connection before it's even established).

@knaeckeKami
Copy link
Collaborator

knaeckeKami commented Nov 18, 2023

Well, at least I'm not going to fix it in the near future. But I'm also not the original contributor and I'm not using the Websocketlinks myself. If someone from the community is willing to fix it or take over maintenance, they are welcome of course.

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

No branches or pull requests

2 participants