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

WebSocketTransport: detect server side completion message. #3337

Open
mkubista opened this issue Feb 16, 2024 · 4 comments
Open

WebSocketTransport: detect server side completion message. #3337

mkubista opened this issue Feb 16, 2024 · 4 comments
Labels
apollo-websockets feature New addition or enhancement to existing solutions networking-stack question Issues that have a question which should be addressed
Milestone

Comments

@mkubista
Copy link

Question

Hi. I have a problem with configuring the WebSocketTransport for Graphql subscriptions. Im have successfully integrated WebSocket and WebSocketTransport into the project. Im able to create subscription. The web socket is being connected. Server is able to send me data and my client code is able to receive data.

The problem is, that when server wants to end subscription, it sends message "{\"id\":\"1\",\"type\":\"complete\"}" And nothing happens. Subscriber is not notified anyhow.
In the WebSocketTransport I found out, that the complete message is in fact ignored, because the subscriptions[id] is never nil. Even if it would be nil the subscriber would be only removed from the internal storage and is not notified.

case .complete:
        if let id = parseHandler.id {
          // remove the callback if NOT a subscription
          if subscriptions[id] == nil {
            subscribers.removeValue(forKey: id)
          }
        } else {
          notifyErrorAllHandlers(WebSocketError(payload: parseHandler.payload,
                                                error: parseHandler.error,
                                                kind: .unprocessedMessage(text)))
        }

I tried to hook up on the raw message closure onText in WebSocket class. There Im able to detect complete message, manually call unsubscribe with apropriate subscriber ID. But again, it only removes it from handler store in transport.
And I didnt find any straight forward option, how to notify that subscriber, that the server decided to end the subscription.
Only way would be to create my own storage for subscriber callback and implement custom OperationMessageIdCreator, because the WebSocketTask is unaccessible. It is final not public, so Im not able to access sequenceNumber

Did I something wrong or there is in fact no simple way, to detect server side end of subscription?

@mkubista mkubista added the question Issues that have a question which should be addressed label Feb 16, 2024
@calvincestari
Copy link
Member

Hi @mkubista - what notification are you wanting to receive and where?

The callback for a subscription event is in response to fetching a query so that callback should only be for new data, not termination of the socket. Similarly WebSocketTransportDelegate only has events for connect, reconnect and error disconnect. There doesn't appear to be any notification we could deliver at the moment that would signal server-side disconnect.

@mkubista
Copy link
Author

I needed anything, that would allow me to somehow react to the event, that subscription has been closed from the server side.
In my case, I have some sort of list of data presented to the user with a small loading animation at the bottom to indicate, that app is still waiting for new data to receive.

On server side, there is a timeout for the data search. When it is passed, the server simply close the subscription as a signal, that there will be no further data updates. So on client the loading animation would disappear and Retry button will appear instead.

But WebSocketTranspor does not have any means to pass this information out of itself. It does the right parsing, but it ends in the part of code, I provided in my first message.

Right now, when Im in a little tight schedule, I solved it by in fact copy-pasting the WebSocketTransport. I created my own transport, with new ErrorKind case (completed). And when complete message arrive, Im passing that error to the appropriate subsciber and after that Im calling unsubscribe on my side (so the |subscription| is correctly removed on my side.

I dont know, if it is the correct way, but server right for my purpose.
I was thinking about new delegate (something like onCompleted) with passing the correct subscriber as parametr of that function, but I need to handle other possible errors anyway.

@calvincestari
Copy link
Member

I'm glad you've found a solution but as you're finding out there are hard limits to the current implementation of websockets and subscriptions. I am hesitant to make that kind of change in main because the 'completed' event is not really an error. Similarly I wouldn't want to call .success with empty data to signal 'complete' because what if the server did send an empty message and the subscription is actually still open (I'm not sure if this is allowed by the ws protocol but it illustrates the point I'm trying to make).

Version 2.0 is really where we'll improve everything networking and websocket subscriptions is in scope for that.

@calvincestari calvincestari added feature New addition or enhancement to existing solutions apollo-websockets networking-stack labels Feb 22, 2024
@calvincestari calvincestari added this to the Release 2.0 milestone Feb 22, 2024
@mkubista
Copy link
Author

Yes I know, that this solution is not very good. Some sort of delegate, which would provide subscriber state change, would be better, but with that, there would be needed some way, to propagate that to the subscriber point of origin anyway.

But Im glad, that there is a Version 2.0 on the way. I will keep that in mind and will follow the progress.
And I noticed, that Im not the only one, who is missing that feature
#3344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apollo-websockets feature New addition or enhancement to existing solutions networking-stack question Issues that have a question which should be addressed
Projects
None yet
Development

No branches or pull requests

2 participants