[web][ff] web socket breaks when changing network

[akosyakov]

At Gitpod we received bug requests that in FireFox (117-118) VS Code Web (1.82) sometimes became unresponsive. See Loom for what unresponsive means.

It is hard to reproduce, but I was able with following:

1. Leave FF tab in the background.
2. Trigger reconnects by toggling your wifi for instance.
3. After a while come back to FF tab.

Debugging web part showed that frontend is keep asking to replay missing messages, but server keeps replaying with the same message. Unfortunately I was not able to debug the server part.

[akosyakov]

It is worth to mention that we observed this issue also without reconnection as well. Would maybe make sense as a safeguard to cycle a web socket if after replay the first replayed message don't satisfy this criteria:

vscode/src/vs/base/parts/ipc/common/ipc.net.ts

Line 1003 in d532fad

if (msg.id !== this._incomingMsgId + 1) {

Otherwise it goes to loop forever?

[akosyakov]

I was able to reproduce it reliably with VS Code Server from sources by toggling VPN exit nodes. I've enabled server side diagnostics and noticed that server gets read and webSocketNodeSocketReadData events after toggling, but there are not more protocolHeaderRead and protocolMessageRead diagnostic traces. This loop does not seem to hit:

vscode/src/vs/base/parts/ipc/common/ipc.net.ts

Line 363 in 63b61dc

while (this._incomingData.byteLength >= this._state.readLen) {

cc @alexdima does it ring any bell? Would it help if i share such traces?

EDIT: I also confirmed that it only happens with FireFox, in case of Chrome it keeps working.

[akosyakov]

Here are traces one from Chrome and another from FireFox with the same test. You can see that at the end of FF log there are no anymore tracing for protocolHeaderRead and protocolMessageRead. (just a note I disabled compression while testing it here but it did not change results)

server.chrome.log
server.ff.log

EDIT: It is interesting that in case of FF protocolHeaderRead does not have a valid message type after changing VPN. If I compare to previous header, it seems everything is shifted, like there are additional bytes which corrupts the header. In my case it is always 801229866761

[akosyakov]

I've tested a workaround that effectively resolves the issue; you can view the changes in our fork [1]. Should I send a pull request?

I've also filed a bug report for Firefox: 1855692.

[alexdima]

Hmm, it's possible Firefox is sending a control opcode that we don't handle well https://datatracker.ietf.org/doc/html/rfc6455#section-5.2

[alexdima]

@akosyakov If you still have an easy repro, could you perhaps log the opcode:

diff --git a/src/vs/base/parts/ipc/node/ipc.net.ts b/src/vs/base/parts/ipc/node/ipc.net.ts
index ed30467961b..e1f14c2361a 100644
--- a/src/vs/base/parts/ipc/node/ipc.net.ts
+++ b/src/vs/base/parts/ipc/node/ipc.net.ts
@@ -390,6 +390,8 @@ export class WebSocketNodeSocket extends Disposable implements ISocket, ISocketT
                                const firstByte = peekHeader.readUInt8(0);
                                const finBit = (firstByte & 0b10000000) >>> 7;
                                const rsv1Bit = (firstByte & 0b01000000) >>> 6;
+                               const opcode = (firstByte & 0b00001111);
+                               console.log(`opcode: ${opcode}`);
                                const secondByte = peekHeader.readUInt8(1);
                                const hasMask = (secondByte & 0b10000000) >>> 7;
                                const len = (secondByte & 0b01111111);

[akosyakov]

@alexdima here is output with FF:

opcode 9 reliably happens when i toggle exit nodes.

Output with Chrome is different:

[jeanp413]

Created a PR fixing it #194436 cc @alexdima

[akosyakov]

Thank you both! I confirm that handling ping control code resolves the issue. @alexdima I verified that @jeanp413's PR work both with Chrome and FireFox.

[alexdima]

Thank you! ❤️

[TylerLeonhardt]

Marking as verified due to @akosyakov's confirmation.