diff --git a/.commit_msg.txt b/.commit_msg.txt new file mode 100644 index 0000000..d2cf766 --- /dev/null +++ b/.commit_msg.txt @@ -0,0 +1,21 @@ +fix: use sub-message decoding for Error fields (not raw bytes) + +The kRPC proto defines error on Response and ProcedureResult as +type Error (a sub-message), not raw bytes. The schema already +declared it that way, so protobufjs was decoding the error as +a nested Error object automatically. + +But our client code was treating response.error as if it were +a Uint8Array of raw bytes (matching the wrong type annotation +we had earlier), and we were trying to decode those bytes as +a second Error message. That double-decode was the source of +the "RPC error: .:" with stray substitute character and the +"index out of range" errors when the bytes happened to look +like other protobuf structures. + +Fix: use the auto-decoded error object directly. response.error +is now { service, name, description, stackTrace } when set, +and we just read those fields. + +This should also let the actual response values decode correctly, +since we were previously mangling the error path. diff --git a/packages/krpc-client/src/client.ts b/packages/krpc-client/src/client.ts index b51e92e..731bd22 100644 --- a/packages/krpc-client/src/client.ts +++ b/packages/krpc-client/src/client.ts @@ -232,23 +232,21 @@ export class KRPCClient { Buffer.from(raw).toString('hex'), ); } + // The kRPC schema defines `error` as a sub-message of type Error + // (not raw bytes), so protobufjs decodes it as a nested object + // automatically — service/name/description/stackTrace are already + // populated when the field is set. const response = decodeMessage<{ - // The error/result fields are `bytes` in the kRPC schema. They - // carry a serialized Error or value sub-message that we have - // to decode on its own — not a pre-decoded object. - error?: Uint8Array; - results: { error?: Uint8Array; value: Uint8Array }[]; + error?: { service: string; name: string; description: string; stackTrace: string }; + results: { + error?: { service: string; name: string; description: string; stackTrace: string }; + value: Uint8Array; + }[]; }>(KRPC.Response, raw); - if (response.error && response.error.length > 0) { - const err = decodeMessage<{ - service: string; - name: string; - description: string; - stackTrace: string; - }>(KRPC.Error, response.error); + if (response.error) { throw new Error( - `RPC error: ${err.service}.${err.name}: ${err.description}` + - (err.stackTrace ? `\n${err.stackTrace}` : ''), + `RPC error: ${response.error.service}.${response.error.name}: ${response.error.description}` + + (response.error.stackTrace ? `\n${response.error.stackTrace}` : ''), ); } if (response.results.length === 0) { @@ -258,16 +256,10 @@ export class KRPCClient { if (!r) { throw new Error('empty response result'); } - if (r.error && r.error.length > 0) { - const err = decodeMessage<{ - service: string; - name: string; - description: string; - stackTrace: string; - }>(KRPC.Error, r.error); + if (r.error) { throw new Error( - `RPC result error: ${err.service}.${err.name}: ${err.description}` + - (err.stackTrace ? `\n${err.stackTrace}` : ''), + `RPC result error: ${r.error.service}.${r.error.name}: ${r.error.description}` + + (r.error.stackTrace ? `\n${r.error.stackTrace}` : ''), ); } return r.value; @@ -297,20 +289,17 @@ export class KRPCClient { }; sendMessage(this.rpcSocket, KRPC.Request, { calls: [addCall] }); const response = decodeMessage<{ - results: { value: Uint8Array; error?: Uint8Array }[]; + results: { + value: Uint8Array; + error?: { service: string; name: string; description: string; stackTrace: string }; + }[]; }>(KRPC.Response, await recvRawMessage(this.rpcSocket)); if (response.results.length === 0) throw new Error('empty AddStream response'); const r0 = response.results[0]; if (!r0) throw new Error('empty AddStream result'); - if (r0.error && r0.error.length > 0) { - const err = decodeMessage<{ - service: string; - name: string; - description: string; - stackTrace: string; - }>(KRPC.Error, r0.error); + if (r0.error) { throw new Error( - `AddStream error: ${err.service}.${err.name}: ${err.description}`, + `AddStream error: ${r0.error.service}.${r0.error.name}: ${r0.error.description}`, ); } const stream = decodeMessage<{ id: number }>(KRPC.Stream, r0.value); @@ -359,21 +348,20 @@ export class KRPCClient { try { const raw = await recvRawMessage(this.streamSocket); const update = decodeMessage<{ - results: { id: number; result: { error?: Uint8Array; value: Uint8Array } }[]; + results: { + id: number; + result: { + error?: { service: string; name: string; description: string; stackTrace: string }; + value: Uint8Array; + }; + }[]; }>(KRPC.StreamUpdate, raw); for (const r of update.results) { - if (r.result.error && r.result.error.length > 0) { - const err = decodeMessage<{ - service: string; - name: string; - description: string; - stackTrace: string; - }>(KRPC.Error, r.result.error); + if (r.result.error) { // eslint-disable-next-line no-console console.warn( - `[krpc-client] stream ${r.id} error: ${err.service}.${err.name}: ${err.description}`, + `[krpc-client] stream ${r.id} error: ${r.result.error.service}.${r.result.error.name}: ${r.result.error.description}`, ); - // Pass empty value so the stream handler sees "no data" rather than crashing for (const h of this.streamHandlers) h(r.id, new Uint8Array()); continue; }