updateConnectingPayload disconnects the web socket connection and doesn't restart it

[cshadek]

Bug report

I have code that calls updateConnectingPayload(...) on the web socket to update the connection params. It always seems to disconnect the web socket and then I can't restart it.

Versions

Please fill in the versions you're currently using:

• apollo-ios SDK version: 1.0.3
• Xcode version: 14.1
• Swift version: 5.7
• Package manager: SPM

Steps to reproduce

print(webSocketTransport.isConnected())  // Returns true
webSocketTransport.updateConnectingPayload(authPayload, reconnectIfConnected: true)
print(webSocketTransport.isConnected()) // Returns false (I would expect this to return true)

If the web socket has not been started this code works fine. Otherwise, it disconnects the web socket and prints the error "websocket is disconnected".

How do I get the websocket to reconnect?

[cshadek]

Is anyone else experiencing this issue?

[cshadek]

@calvincestari any chance you know anything about this? Is there a good temporary solution?

My team uses an OAuth based authentication with a refresh token, so we really need to be able to update the connecting payload.

[calvincestari]

@cshadek, I haven't looked into this yet.

Was this working before or a new bug?

[cshadek]

@calvincestari I think it used to work, but maybe that was before 1.0. We only added subscriptions recently, so I'm honestly not sure. Also, it seems that it was an issue around a year ago in #2129 as well and got fixed then. Maybe 1.0 caused an issue?

[calvincestari]

If you can narrow it down to a time when it worked, it would help to find any regression bug. I won't know more until we can try reproduce it.

[cshadek]

I noticed in the tests that the code closes the web socket connection before updating the payload and then calls initserver() again.

Is this the best way to do it? I just assumed the updateConnectingPayload() would take care of that.

self.webSocketTransport.closeConnection()
self.webSocketTransport.updateConnectingPayload(["Authorization": "UpdatedToken"])
self.webSocketTransport.initServer()

[calvincestari]

Is this the best way to do it? I just assumed the updateConnectingPayload() would take care of that.

This part of the framework predates my time at Apollo. I'll have to dig around and see what the correct behaviour should be; I agree that reconnectIfConnected: true handling it for you is a reasonable expectation.

[cshadek]

So I recently updated our backend library (we're using Pioneer - it's basically a valiant attempt at ApolloServer for Swift).

And now the uploadConnectingPayload seems to work most of the time. Not sure what changed. Perhaps it was just a backend issue?

We're still having issues with pausing and resuming the web socket connection though.

[calvincestari]

We're still having issues with pausing and resuming the web socket connection though.

Is that captured in a different issue?

[cshadek]

Is that captured in a different issue?

Not that I can tell. I created one on Pioneer, but not here yet. I could create another issue

[calvincestari]

I could create another issue

You could always change the title of this one and add the updated problem description in another comment.

[cshadek]

@calvincestari so I did some more investigation. It seems that resumeWebSocketConnection doesn't happen right away. And I've been re-adding subscriptions right after it resumes. I've also been logging isConnected() right after calling resume.

For now, a fix that seems to work is adding a delay after calling resume like the following:

Task {
  print(transport.isConnected()) // prints false
  transport.resumeWebSocketConnection(autoReconnect: true)
  try await Task.sleep(nanoseconds: 1_000_000_000)
  print(transport.isConnected()) // prints true
} 

Maybe there should be a version of resumeWebSocketConnection that includes a callback so code can be run after the websocket is connected/ reconnected? This should probably be another issue/ feature request. Otherwise, if the plan is to add async await support to Apollo, then obviously you wouldn't need the callback and could just make these functions async.

[calvincestari]

Otherwise, if the plan is to add async await support to Apollo, then obviously you wouldn't need the callback and could just make these functions async.

We will definitely be adding support for the new Swift Concurrency features in v2.0.

[calvincestari]

I think it comes down to this line in the connection sequence.

It's not great to have to introduce a delay into your code, it's pretty horrible really. I'm hesitant to add a completion block to the reconnect API without looking into that a lot more. All of this code is within scope for 2.0 but realistically that's a while away from being planned/started.

[cshadek]

I think it comes down to this line in the connection sequence.

It's not great to have to introduce a delay into your code, it's pretty horrible really. I'm hesitant to add a completion block to the reconnect API without looking into that a lot more. All of this code is within scope for 2.0 but realistically that's a while away from being planned/started.

Haha yeah it's not ideal, but my understanding is Task.sleep is non blocking and it's not on the main thread, so I wouldn't think it would cause too many issues.

It's really a short term fix until v2. But, I agree a completion block would be better code.

As side note, for my convenience I actually wrapped most of the Apollo API in async/await code, so hopefully it will be easy to move to v2 when that happens!