After receiving web socket error message "second and beyond of fragment message must be a continue frame", web socket fails until Apollo client recreated

[dfed]

Summary

I've seen a couple internal reports in the last week where my Apollo client's web socket fails with the error:

second and beyond of fragment message must be a continue frame

Seems like a transport issue, which should be transient. However, when a client gets into this state, it continues to fail to successfully receive a web socket event due to this error. Any reconnect, resubscription, or new subscription attempts result in the same error. It looks like a network glitch completely poisons the apollo web socket client

Version

1.23.0

Steps to reproduce the behavior

Unknown.

Logs

Anything else?

I'm very new to this code so forgive me if I'm off base: I do see a hint as to what could be causing the repeated failures. processOneRawMessage reads the last response in the read stack to determine what to expect from the next response. When it fails, it calls doDisconnect, which does not reset the readStack, which means that the readStack will always have a last response, which means (or at least I think it means) we'll always re-enter this second and beyond of fragment message must be a continue frame error loop.

[calvincestari]

Hi @dfed - it's the first time I've seen this error so it'll take a little while to try figure out what's going on; your comment about the readStack seems plausible though. That code is from StarScream v3 which we've vendored into our codebase.

I think trying to get a test written that reproduces this error is the best path forwards although I'm unsure how testable that websocket code actually is. I'll need to find time to try a few things.

[dfed]

fwiw, I've been running a fork of Apollo these last few months with the following change, and it's been working well:

diff --git a/Sources/ApolloWebSocket/DefaultImplementation/WebSocket.swift b/Sources/ApolloWebSocket/DefaultImplementation/WebSocket.swift
index 60de0f8c..cf488004 100644
--- a/Sources/ApolloWebSocket/DefaultImplementation/WebSocket.swift
+++ b/Sources/ApolloWebSocket/DefaultImplementation/WebSocket.swift
@@ -590,7 +590,7 @@ public final class WebSocket: NSObject, WebSocketClient, StreamDelegate, WebSock
   private func dequeueInput() {
     while !inputQueue.isEmpty {
       autoreleasepool {
-        let data = inputQueue[0]
+        let data = inputQueue.remove(at: 0)
         var work = data
         if let buffer = fragBuffer {
           var combine = NSData(data: buffer) as Data
@@ -605,7 +605,6 @@ public final class WebSocket: NSObject, WebSocketClient, StreamDelegate, WebSock
         } else {
           processRawMessagesInBuffer(buffer, bufferLen: length)
         }
-        inputQueue = inputQueue.filter{ $0 != data }
       }
     }
   }
@@ -1013,7 +1012,7 @@ public final class WebSocket: NSObject, WebSocketClient, StreamDelegate, WebSock
           }
         }
       }
-      readStack.removeLast()
+      _ = readStack.popLast() // we need to remove the last since we just processed it, but there is no locking between here and doDisconnect, so avoid crashing if the stack is empty.
       return true
     }
     return false
@@ -1111,6 +1110,7 @@ public final class WebSocket: NSObject, WebSocketClient, StreamDelegate, WebSock
   private func doDisconnect(_ error: (any Error)?) {
     serialQueue.sync {
       guard !self.didDisconnect else { return }
+      readStack = []
       self.didDisconnect = true
       self.isConnecting = false
       self.mutex.lock()

[calvincestari]

I think these changes make sense @dfed, thank you. I've rolled it up into PR #850 and it'll go out in the next release.

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.