hasura / go-graphql-client

Package graphql provides a GraphQL client implementation.
MIT License
405 stars 94 forks source link

implement graphql-ws protocol #38

Closed hgiasac closed 1 year ago

hgiasac commented 2 years ago

The subscriptions-transport-ws protocol is no longer maintained. It's better to have an option to support graphql-ws protocol

jflambert commented 2 years ago

Hi @hgiasac could you tell me what this issue means for regular users?

Should we expect breaking changes when you "switch over" to the other spec?

hgiasac commented 2 years ago

It depends on the GraphQL server that supports which protocol. Hasura GraphQL Engine still supports both protocols for example. The client can support both protocols as well. We can decide which protocol is used by some variable. The default protocol can be subscriptions-transport-ws. There shouldn't be any breaking change.

jflambert commented 2 years ago

awesome. I'm a hasura user yes.

andig commented 1 year ago

The subscriptions-transport-ws protocol is no longer maintained. It's better to have an option to support graphql-ws protocol

You've got me confused. According to https://developer.tibber.com/docs/overview#breaking-websocket-change, it is graphql-ws that is no longer maintained?

Now, connecting to wss://websocket-api.tibber.com/v1-beta/gql/subscriptions using graphql-ws I get:

failed to WebSocket dial: expected handshake response status code 101 but got 503. retry in second... client

Using graphql-transport-ws and

options := &websocket.DialOptions{
    Subprotocols: []string{"graphql-transport-ws"},
    HTTPClient: sc.websocketOptions.HTTPClient,
}

I get:

2022/11/16 19:16:25 {"type":"connection_terminate"} client
2022/11/16 19:16:25 {"type":"connection_init","payload":{"token":"5K4MVS-OjfWhK_4yrjOlFe1F6kJXPVf7eQYggo8ebAE"}} client
2022/11/16 19:16:25 {"id":"869c7de7-eb08-4b1d-93d8-bf747d674bfb","type":"start","payload":{"query":"subscription ($homeId:ID!){liveMeasurement(homeId: $homeId){power,lastMeterConsumption,lastMeterProduction,currentL1,currentL2,currentL3}}","variables":{"homeId":"96a14971-525a-4420-aae9-e5aedaa129ff"}}} client
.2022/11/16 19:16:25 failed to read JSON message: failed to get reader: received close frame: status = StatusCode(4400) and reason = "Invalid message received". Retry connecting... client

which I would read as "connection successful, transfer not".

So I'm wondering: what is missing here and how to proceed?

andig commented 1 year ago

More confusion: https://github.com/apollographql/subscriptions-transport-ws is archived and documents, that graphql-ws is the current protocol (which makes Tibber's docs wrong). graphql-ws is also hasura's sub protocol. However, Tibber connection fails as shown above, plus source code comment says:

// Subscription transport follow Apollo's subscriptions-transport-ws protocol specification
// https://github.com/apollographql/subscriptions-transport-ws/blob/master/PROTOCOL.md

So what is the actual implemented protocol version as of now?

chuongle commented 1 year ago

Has anyone get graphql-transport-ws protocol working? I tried to follow the README but I kept getting error cannot create context from nil parent

andig commented 1 year ago

@hgiasac any chance for an update? It seems Tibber integration (https://developer.tibber.com/docs/overview#breaking-websocket-change) is broken due to missing graphql-transport-ws. However, https://github.com/hasura/go-graphql-client/issues/38#issue-1292826306 states exactly the opposite.

Could you kindly clarify the status of graphql-transport-ws support? I'd be happy to help out with testing!

hgiasac commented 1 year ago

@andig the library doesn't support graphql-ws protocol yet. So far it only supports the legacy subscriptions-transport-ws protocol. If you can wait for few days, I will create a PR for the new protocol and you can test it

andig commented 1 year ago

@hgiasac wow- that sounds great! I've already started to take a first look but am already stuck at receiving the connection_init ACK message. Happy to way and happy to test anything, even if half-finished, you want to share.

terjesannum commented 1 year ago

@andig I understand that you are using the graphql subscription service in the Tibber API. It seems that this service requires the connection_init to be ack'ed before the subscription can be started. This library seems to send the subscription start command right away, and the Tibber server isn't finished authenticating the client and denies the request. @hgiasac Can you take a look at that too?

I did a quick hack to get it work by just adding a short pause. You can see my changes here: https://github.com/terjesannum/go-graphql-client/commit/96d328ce0c838e8bfa0992518cdb2648a7cc92e4

andig commented 1 year ago

It seems that this service requires the connection_init to be ack'ed before the subscription can be started.

That's my understanding of graphql-ws

This library seems to send the subscription start command right away, and the Tibber server isn't finished authenticating the client and denies the request

Exactly. I've tried the same, but never got the ACK from the server. Maybe a specific Tibber issue, I'll probably need to setup a GraphQL server for local testing.

hgiasac commented 1 year ago

@andig sorry for the late reply. This is the PR for the graphql-ws protocol support. https://github.com/hasura/go-graphql-client/pull/67

Can you please verify if it works with Tibber? So far it can works with Hasura graphql engine (https://github.com/hasura/go-graphql-client/tree/support-graphql-ws-protocol/example/hasura)

andig commented 1 year ago

Confirmed working. I can subscribe and retrieve data. Didn't do any long-running tests. Much appreciated!

I'm sorry, reverting my comment. Tbo, I'm not sure what the Tibber API is doing right now and hence not sure if my test is valid. Will continue investigating.

hgiasac commented 1 year ago

@andig I have fixed some error and add the example for Tibber here

Please update the package to the latest commit and verify

andig commented 1 year ago

LGTM! Seems my test failed because Tibber- when using graphql-ws- will simply not reply to the handshake when the user agent string doesn't have the expected format. With the head you've added things are fine. Much appreciated!