From cdc24621c09e830f2f47294fba08ade6c4dcac2a Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 3 Sep 2026 05:52:42 -0400 Subject: [PATCH] fix(relay): stop self-closing the control socket on unknown messages The desktop control client tore its own relay control WebSocket down with code 4401 "unknown control message" for any well-formed control frame it did not recognize. handleMessage() funneled everything that was not ping / conn-open / drain / a tracked request reply into failProtocol('unknown control message'), which closes the socket and orphans the origin. Three real frames hit that branch: - A relay reply that arrives after the desktop's 10s request deadline already deleted the pending entry. Relay control operations run DB transactions that can exceed 10s under load, so resolveMessage() finds no waiter and returns false. - A control-error carrying no reqId (or an unknown one), including the relay's own 'unknown_control_message' reply to a host command it could not route. - A newer relay's opcode that this build predates. Fleet telemetry shows ~15 of these closes per day across app versions 1.4.175..1.4.197, so it is version-agnostic. The self-close was also far more costly than the message that caused it: the relay session dropped to 'orphaned' and answered the phone with HOST_OFFLINE (4404) for the orphan grace window, then the desktop had to re-register through the director's 503 reconnect throttle, stretching a single stray frame into minutes of mobile downtime. Per docs/reference/remote-wire-compatibility.md Rule 2, an unknown but well-formed control frame must be dropped, not treated as fatal. Log and ignore it; malformed JSON, binary frames, and messages before activation still close as protocol violations. Adds unit tests for the unknown-opcode drop, the timed-out-reply drop, and the preserved malformed-frame teardown. --- .../relay/relay-control-client.test.ts | 45 +++++++++++++++++++ .../runtime/relay/relay-control-client.ts | 11 ++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/main/runtime/relay/relay-control-client.test.ts b/src/main/runtime/relay/relay-control-client.test.ts index 2975b67e631..d235f0ebacd 100644 --- a/src/main/runtime/relay/relay-control-client.test.ts +++ b/src/main/runtime/relay/relay-control-client.test.ts @@ -4,6 +4,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import nacl from 'tweetnacl' import { WebSocketServer, type WebSocket } from 'ws' import type { E2EEKeypair } from '../e2ee-keypair' +import { MOBILE_RELAY_CLOSE_CODE } from '../../../shared/mobile-relay-close-codes' import { RelayControlClient } from './relay-control-client' const encoder = new TextEncoder() @@ -550,4 +551,48 @@ describe('RelayControlClient scripted-socket lifecycle', () => { vi.advanceTimersByTime(91_000) expect(client.isLive()).toBe(false) }) + + it('ignores an unrecognized control message without closing the active control', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { client, socket, onClose } = scriptedControl() + await client.connect() + expect(client.isLive()).toBe(true) + + // A newer relay opcode the desktop schema does not know. Rule 2 of + // remote-wire-compatibility: an unknown-but-well-formed frame is dropped, + // never fatal to a live control. + socket.deliver({ type: 'relay-hint', v: 2, hint: 'future-feature' }) + + expect(client.isLive()).toBe(true) + expect(socket.readyState).toBe(1) + expect(onClose).not.toHaveBeenCalled() + warn.mockRestore() + }) + + it('ignores a reply whose request already timed out instead of self-closing', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { client, socket, onClose } = scriptedControl() + await client.connect() + + // A relay control-error carrying a reqId with no live waiter — e.g. a late + // reply that arrived after the desktop's request deadline deleted it, or the + // relay's no-op error for a command it could not route. Must not be fatal. + socket.deliver({ type: 'control-error', reqId: 'expired-req', code: 'unknown_control_message' }) + + expect(client.isLive()).toBe(true) + expect(socket.readyState).toBe(1) + expect(onClose).not.toHaveBeenCalled() + warn.mockRestore() + }) + + it('still tears down a malformed (non-JSON) control frame', async () => { + const { client, socket, onClose } = scriptedControl() + await client.connect() + + socket.emit('message', 'not-json{', false) + + expect(client.isLive()).toBe(false) + expect(socket.readyState).toBe(3) + expect(onClose).toHaveBeenCalledWith(MOBILE_RELAY_CLOSE_CODE.BAD_OUTER_CREDENTIAL) + }) }) diff --git a/src/main/runtime/relay/relay-control-client.ts b/src/main/runtime/relay/relay-control-client.ts index 76816c81a3a..231ce2e139e 100644 --- a/src/main/runtime/relay/relay-control-client.ts +++ b/src/main/runtime/relay/relay-control-client.ts @@ -234,7 +234,16 @@ export class RelayControlClient { if (this.requests.resolveMessage(message)) { return } - this.failProtocol('unknown control message') + // Wire-compat (docs/reference/remote-wire-compatibility.md, Rule 2): a + // well-formed control message we do not recognize must be dropped, never + // fatal. It is either a newer relay's opcode this build predates, or a reply + // whose request already timed out and has no waiter (relay control ops run DB + // transactions that can exceed the request deadline under load). Self-closing + // here forced an orphaned relay session that answered the phone with + // HOST_OFFLINE for the orphan-grace window plus the director's reconnect + // throttle — minutes of outage from a message that could simply be ignored. + const messageType = typeof message.type === 'string' ? message.type : 'unknown' + console.warn(`[relay] ignoring unrecognized control message type=${messageType}`) } private handleProofMessage(message: Record): void {