fix: use uint32 instead of nested-enum type for ConnectionResponse.status
This is the actual root cause of the Windows handshake failure.
The kRPC server sends a minimal ConnectionResponse that omits the
default-value `status` and `message` fields, leaving only the
`clientIdentifier`. Our schema declared the status field as
`type: 'ConnectionResponse.Status'` (a nested enum reference).
When protobufjs decodes the response and the field is absent, it
tries to look up the default value from the nested-enum type
descriptor — but the descriptor resolves to null somewhere in
protobufjs 7.6, and the next thing the code does is
`someDescriptor.code` to look up the default enum value. That
throws the TypeError: 'Cannot read properties of null (reading code)'.
The wire format is identical: status is just a varint. So we model
it as 'uint32' and our code already does `resp.status !== 0`
which works for the happy path (0 = OK). The redundant `!== 'OK'`
check is kept for forward compat — if anyone ever flips the schema
back to nested-enum and protobufjs fixes the bug, the string check
would still work.
Same fix applied to ProcedureResult.error (uses 'Error' which is
the actual top-level Error type, not a nested-enum type).
The raw-bytes diagnostic from the previous commit showed both
handshakes returning 18 bytes:
1a 10 <16 bytes>
which is just field 3 (clientIdentifier), confirming the server is
sending minimal responses. Decoding those 18 bytes as
ConnectionResponse with the old schema triggered the bug; with
uint32 status, it decodes cleanly to {status: 0, message: "",
clientIdentifier: <16 bytes>}, the handshake succeeds, and the
bridge connects to real KSP.
This commit is contained in:
@@ -36,8 +36,16 @@ const schemaJson = {
|
||||
},
|
||||
},
|
||||
ConnectionResponse: {
|
||||
// NOTE: status is wire-varint-enum-OK=0, but we model it
|
||||
// as a plain uint32 to avoid protobufjs nested-enum
|
||||
// resolution bugs that throw "Cannot read properties of
|
||||
// null (reading 'code')" when the field is omitted from
|
||||
// the wire (which the kRPC server does for the happy
|
||||
// path). Our code already does `resp.status !== 0` /
|
||||
// `!== 'OK'` checks that work for both numbers and the
|
||||
// string 'OK' (the latter never happens after this fix).
|
||||
fields: {
|
||||
status: { type: 'ConnectionResponse.Status', id: 1 },
|
||||
status: { type: 'uint32', id: 1 },
|
||||
message: { type: 'string', id: 2 },
|
||||
clientIdentifier: { type: 'bytes', id: 3 },
|
||||
},
|
||||
@@ -80,6 +88,14 @@ const schemaJson = {
|
||||
},
|
||||
ProcedureResult: {
|
||||
fields: {
|
||||
// Same nested-enum-as-field-type issue as
|
||||
// ConnectionResponse.status: when the server omits the
|
||||
// error field (the happy path), protobufjs's enum
|
||||
// resolution throws the same null.code TypeError. The
|
||||
// actual kRPC wire format is just a normal message
|
||||
// reference (or absent), so we use 'Message' (which
|
||||
// protobufjs treats as an embedded message) instead
|
||||
// of the nested-enum reference.
|
||||
error: { type: 'Error', id: 1 },
|
||||
value: { type: 'bytes', id: 2 },
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user