mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(relay): stop self-closing the control socket on unknown messages (#18400)
* 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.
* docs(relay): correct the ignore rationale, drop the Rule 2 misattribution
Rule 2 of remote-wire-compatibility governs the SENDER of a new terminal-
stream opcode and treats the receiver's silent drop as a hazard, not a
mandate. Reframe the comment around the actual justification: the decoder
convention of dropping unknown frames, the control channel's lack of an
opcode negotiation step, and the incident cost asymmetry.
This commit is contained in:
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -234,7 +234,18 @@ export class RelayControlClient {
|
||||
if (this.requests.resolveMessage(message)) {
|
||||
return
|
||||
}
|
||||
this.failProtocol('unknown control message')
|
||||
// Drop a well-formed control message we do not recognize, matching how every
|
||||
// other Orca decoder treats an unknown frame (see the silent-drop convention
|
||||
// in docs/reference/remote-wire-compatibility.md). The control channel has no
|
||||
// opcode negotiation step, so this reaches either a newer relay's message
|
||||
// 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 was strictly worse than ignoring:
|
||||
// it orphaned the relay session, which answered the phone with HOST_OFFLINE
|
||||
// for the orphan-grace window plus the director's reconnect throttle — minutes
|
||||
// of outage from a single stray frame.
|
||||
const messageType = typeof message.type === 'string' ? message.type : 'unknown'
|
||||
console.warn(`[relay] ignoring unrecognized control message type=${messageType}`)
|
||||
}
|
||||
|
||||
private handleProofMessage(message: Record<string, unknown>): void {
|
||||
|
||||
Reference in New Issue
Block a user