From b98a3b07361eff0636571e03e2aa1cf1041f1e22 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 16:53:56 +1100 Subject: [PATCH 1/8] feat: toError and fromError working as intended --- src/RPCClient.ts | 17 +-- src/RPCServer.ts | 49 +++++---- src/errors.ts | 146 ++++++++++---------------- src/types.ts | 6 ++ src/utils.ts | 251 +++++++++++++++++++++++++++----------------- tests/RPC.test.ts | 126 ++++------------------ tests/utils.test.ts | 8 ++ tests/utils.ts | 17 ++- 8 files changed, 288 insertions(+), 332 deletions(-) diff --git a/src/RPCClient.ts b/src/RPCClient.ts index abaed2c..b237848 100644 --- a/src/RPCClient.ts +++ b/src/RPCClient.ts @@ -13,8 +13,9 @@ import type { ClientManifest, RPCStream, JSONRPCResponseResult, + ToError, } from './types'; -import type { ErrorRPC } from './errors'; +import { JSONRPCErrorCode } from './errors'; import Logger from '@matrixai/logger'; import { Timer } from '@matrixai/timer'; import * as middleware from './middleware'; @@ -28,7 +29,7 @@ class RPCClient { protected idGen: IdGen; protected logger: Logger; protected streamFactory: StreamFactory; - protected toError?: typeof utils.toError; + protected toError: ToError; protected middlewareFactory: MiddlewareFactory< Uint8Array, JSONRPCRequest, @@ -86,7 +87,7 @@ class RPCClient { middlewareFactory = middleware.defaultClientMiddlewareWrapper(), streamKeepAliveTimeoutTime = Infinity, logger, - toError, + toError = utils.toError, idGen = () => Promise.resolve(null), }: { manifest: M; @@ -100,10 +101,7 @@ class RPCClient { streamKeepAliveTimeoutTime?: number; logger?: Logger; idGen?: IdGen; - toError?: ( - errorData: JSONValue, - metadata: Record, - ) => ErrorRPC; + toError?: ToError; }) { this.idGen = idGen; this.callerTypes = utils.getHandlerTypes(manifest); @@ -472,7 +470,10 @@ class RPCClient { ...(rpcStream.meta ?? {}), command: method, }; - throw utils.toError(messageValue.error, metadata); + if (messageValue.error.code === JSONRPCErrorCode.RPCRemote) { + throw this.toError(JSON.parse(messageValue.error.data as string), metadata); + } + throw errors.ErrorRPCProtocol.fromJSON(messageValue.error); } leadingMessage = messageValue; } catch (e) { diff --git a/src/RPCServer.ts b/src/RPCServer.ts index 513fcd4..42bee73 100644 --- a/src/RPCServer.ts +++ b/src/RPCServer.ts @@ -15,6 +15,7 @@ import type { UnaryHandlerImplementation, RPCStream, MiddlewareFactory, + FromError, } from './types'; import { ReadableStream, TransformStream } from 'stream/web'; import Logger from '@matrixai/logger'; @@ -59,8 +60,8 @@ class RPCServer { protected defaultTimeoutMap: Map = new Map(); protected handlerTimeoutTime: number; protected activeStreams: Set> = new Set(); - protected fromError: (error: errors.ErrorRPC) => JSONValue; - protected filterSensitive: (key: string, value: any) => any; + protected fromError: FromError; + protected replacer?: (key: string, value: any) => any; protected middlewareFactory: MiddlewareFactory< JSONRPCRequest, Uint8Array, @@ -94,7 +95,7 @@ class RPCServer { logger, idGen = () => Promise.resolve(null), fromError = utils.fromError, - filterSensitive = utils.filterSensitive, + replacer, }: { middlewareFactory?: MiddlewareFactory< JSONRPCRequest, @@ -105,14 +106,14 @@ class RPCServer { handlerTimeoutTime?: number; logger?: Logger; idGen?: IdGen; - fromError?: (error: errors.ErrorRPC) => JSONValue; - filterSensitive?: (key: string, value: any) => any; + fromError?: FromError; + replacer?: (key: string, value: any) => any; }) { this.idGen = idGen; this.middlewareFactory = middlewareFactory; this.handlerTimeoutTime = handlerTimeoutTime; - this.fromError = fromError ?? utils.fromError; - this.filterSensitive = filterSensitive ?? utils.filterSensitive; + this.fromError = fromError; + this.replacer = replacer; this.logger = logger ?? new Logger(this.constructor.name); } @@ -196,7 +197,7 @@ class RPCServer { const handlerPs = new Array>(); if (force) { for await (const [activeStream] of this.activeStreams.entries()) { - if (force) activeStream.cancel(new errors.ErrorRPCStopping()); + if (force) activeStream.cancel(reason); handlerPs.push(activeStream); } await Promise.all(handlerPs); @@ -319,11 +320,17 @@ class RPCServer { } controller.enqueue(value); } catch (e) { - const rpcError: JSONRPCError = { - code: e.exitCode ?? errors.JSONRPCErrorCode.InternalError, - message: e.description ?? '', - data: JSON.stringify(this.fromError(e), this.filterSensitive), - }; + let rpcError: JSONRPCError + if (e instanceof errors.ErrorRPCProtocol) { + rpcError = e.toJSON(); + } + else { + rpcError = { + code: errors.JSONRPCErrorCode.RPCRemote, + message: e?.message ?? '', + data: JSON.stringify(this.fromError(e), this.replacer), + }; + } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', error: rpcError, @@ -576,11 +583,17 @@ class RPCServer { { signal: abortController.signal, timer }, ); } catch (e) { - const rpcError: JSONRPCError = { - code: e.exitCode ?? errors.JSONRPCErrorCode.InternalError, - message: e.description ?? '', - data: JSON.stringify(this.fromError(e), this.filterSensitive), - }; + let rpcError: JSONRPCError + if (e instanceof errors.ErrorRPCProtocol) { + rpcError = e.toJSON(); + } + else { + rpcError = { + code: errors.JSONRPCErrorCode.RPCRemote, + message: e?.message ?? '', + data: JSON.stringify(this.fromError(e), this.replacer), + }; + } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', error: rpcError, diff --git a/src/errors.ts b/src/errors.ts index f4c8af0..9c51885 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,6 +1,7 @@ import type { Class } from '@matrixai/errors'; -import type { JSONValue } from '@/types'; +import type { JSONRPCError, JSONValue } from '@/types'; import { AbstractError } from '@matrixai/errors'; +import { JSONRPCErrorCode, rpcProtocolErrors } from './utils'; class ErrorRPC extends AbstractError { static description = 'RPC Error'; @@ -16,35 +17,54 @@ class ErrorRPCServerNotRunning extends ErrorRPC { static description = 'RPCServer is not running'; } -// Protocol Errors - -const enum JSONRPCErrorCode { - ParseError = -32700, - InvalidRequest = -32600, - MethodNotFound = -32601, - InvalidParams = -32602, - InternalError = -32603, - HandlerNotFound = -32000, - RPCStopping = -32001, - RPCMessageLength = -32003, - RPCMissingResponse = -32004, - RPCOutputStreamError = -32005, - RPCRemote = -32006, - RPCStreamEnded = -32007, - RPCTimedOut = -32008, - RPCConnectionLocal = -32010, - RPCConnectionPeer = -32011, - RPCConnectionKeepAliveTimeOut = -32012, - RPCConnectionInternal = -32013, - MissingHeader = -32014, - HandlerAborted = -32015, - MissingCaller = -32016, +/** + * This is an internal error, it should not reach the top level. + */ +class ErrorRPCHandlerFailed extends ErrorRPC { + static description = 'Failed to handle stream'; +} + +class ErrorRPCCallerFailed extends ErrorRPC { + static description = 'Failed to call stream'; } abstract class ErrorRPCProtocol extends ErrorRPC { static error = 'RPC Protocol Error'; code: number; - type: string; + + public static fromJSON>( + json: any, + ): InstanceType { + if ( + typeof json !== 'object' || + typeof json.code !== 'number' || + typeof json.message !== 'string' + ) { + throw new TypeError(`Cannot decode JSON to ${this.name}`); + } + + const errorC = rpcProtocolErrors[json.code]; + + if (errorC == null) { + throw new TypeError(`Cannot decode JSON to ${this.name}`); + } + + const e: InstanceType = new errorC(json.message); + e.data = json.data; + e.timestamp = e.data.timestamp; + + return e; + } + public toJSON(): JSONRPCError { + return { + code: this.code, + message: this.message, + data: { + timestamp: this.timestamp.toJSON(), + ...this.data + } + } + } } class ErrorRPCParse extends ErrorRPCProtocol { @@ -52,22 +72,15 @@ class ErrorRPCParse extends ErrorRPCProtocol { code = JSONRPCErrorCode.ParseError; } -class ErrorRPCStopping extends ErrorRPCProtocol { - static description = 'RPC is stopping'; - code = JSONRPCErrorCode.RPCStopping; +class ErrorRPCInvalidParams extends ErrorRPCProtocol { + static description = 'Invalid paramaters provided to RPC'; + code = JSONRPCErrorCode.InvalidParams; } -/** - * This is an internal error, it should not reach the top level. - */ -class ErrorRPCHandlerFailed extends ErrorRPCProtocol { - static description = 'Failed to handle stream'; - code = JSONRPCErrorCode.HandlerNotFound; -} -class ErrorRPCCallerFailed extends ErrorRPCProtocol { - static description = 'Failed to call stream'; - code = JSONRPCErrorCode.MissingCaller; +class ErrorRPCStopping extends ErrorRPCProtocol { + static description = 'RPC is stopping'; + code = JSONRPCErrorCode.RPCStopping; } class ErrorMissingCaller extends ErrorRPCProtocol { @@ -102,61 +115,7 @@ class ErrorRPCOutputStreamError extends ErrorRPCProtocol { class ErrorRPCRemote extends ErrorRPCProtocol { static description = 'Remote error from RPC call'; static message: string = 'The server responded with an error'; - metadata: JSONValue | undefined; - - constructor({ - metadata, - message, - options, - }: { - metadata?: JSONValue; - message?: string; - options?: any; - } = {}) { - super(message, options); - this.metadata = metadata; - this.code = JSONRPCErrorCode.RPCRemote; - this.data = options?.data; - this.type = this.constructor.name; - this.message = message || ErrorRPCRemote.message; - } - - public static fromJSON>( - this: T, - json: any, - ): InstanceType { - if ( - typeof json !== 'object' || - json.type !== this.name || - typeof json.data !== 'object' || - typeof json.data.message !== 'string' || - isNaN(Date.parse(json.data.timestamp)) || - typeof json.data.metadata !== 'object' || - typeof json.data.data !== 'object' || - ('stack' in json.data && typeof json.data.stack !== 'string') - ) { - throw new TypeError(`Cannot decode JSON to ${this.name}`); - } - - // Here, you can define your own metadata object, or just use the one from JSON directly. - const parsedMetadata = json.data.metadata; - - const e = new this(parsedMetadata, json.data.message, { - timestamp: new Date(json.data.timestamp), - data: json.data.data, - cause: json.data.cause, - }); - e.stack = json.data.stack; - return e; - } - public toJSON(): any { - return { - type: this.name, - data: { - description: this.description, - }, - }; - } + code = JSONRPCErrorCode.RPCRemote; } class ErrorRPCStreamEnded extends ErrorRPCProtocol { @@ -207,6 +166,7 @@ export { ErrorRPCProtocol, ErrorRPCStopping, ErrorRPCParse, + ErrorRPCInvalidParams, ErrorRPCHandlerFailed, ErrorRPCMessageLength, ErrorRPCMissingResponse, diff --git a/src/types.ts b/src/types.ts index 7ed1295..8fe6324 100644 --- a/src/types.ts +++ b/src/types.ts @@ -348,6 +348,10 @@ type HandlerTypes = T extends Handler< } : never; +type FromError = (error: any) => JSONValue; + +type ToError = (errorData: JSONValue, metadata: JSONValue) => any; + export type { IdGen, JSONRPCRequestMessage, @@ -376,4 +380,6 @@ export type { POJO, PromiseDeconstructed, HandlerTypes, + FromError, + ToError }; diff --git a/src/utils.ts b/src/utils.ts index f3b5b51..75068a8 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,13 +16,10 @@ import type { import { TransformStream } from 'stream/web'; import { JSONParser } from '@streamparser/json'; import { AbstractError } from '@matrixai/errors'; -import { JsonableValue } from 'ts-jest'; import { ErrorRPCRemote, ErrorRPC, - ErrorRPCMethodNotImplemented, ErrorRPCConnectionInternal, - JSONRPCErrorCode, ErrorRPCStopping, ErrorRPCMessageLength, ErrorRPCParse, @@ -36,6 +33,8 @@ import { ErrorRPCConnectionKeepAliveTimeOut, ErrorMissingHeader, ErrorMissingCaller, + ErrorRPCProtocol, + ErrorRPCInvalidParams, } from './errors'; import * as rpcErrors from './errors'; @@ -235,21 +234,50 @@ function parseJSONRPCMessage( ); } /** - * Serializes an ErrorRPC instance into a JSONValue object suitable for RPC. - * @param {ErrorRPC} error - The ErrorRPC instance to serialize. - * @param {any} [id] - Optional id for the error object in the RPC response. + * Serializes an Error instance into a JSONValue object suitable for RPC. + * @param {Error} error - The Error instance to serialize. * @returns {JSONValue} The serialized ErrorRPC instance. */ function fromError( - errorin: rpcErrors.ErrorRPCProtocol, - id?: any, + error: any, ): JSONValue { - const error: { [key: string]: JSONValue } = { - errorCode: errorin.code, - message: errorin.message, - data: errorin.data, - type: errorin.constructor.name, - }; + // TODO: Linked-List traversal must be done iteractively rather than recusively to prevent stack overflow. + switch (typeof error) { + case "symbol": + case "bigint": + case "function": + throw TypeError(`${error} cannot be serialized`); + } + + if (error instanceof Error) { + const cause = fromError(error.cause); + const timestamp: string = ((error as any).timestamp ?? new Date()).toJSON(); + if (error instanceof AbstractError) { + return error.toJSON(); + } + else if (error instanceof AggregateError) { + // AggregateError has an `errors` property + return { + type: error.constructor.name, + errors: error.errors.map(fromError), + message: error.message, + stack: error.stack, + timestamp, + cause + }; + } + + // If it's some other type of error then only serialise the message and + // stack (and the type of the error) + return { + type: error.name, + message: error.message, + stack: error.stack, + timestamp, + cause, + }; + } + return error; } @@ -257,7 +285,7 @@ function fromError( * Error constructors for non-Polykey rpcErrors * Allows these rpcErrors to be reconstructed from RPC metadata */ -const standardErrors = { +const standardErrors: { [key: string]: typeof Error | typeof AggregateError | typeof AbstractError } = { Error, TypeError, SyntaxError, @@ -267,59 +295,72 @@ const standardErrors = { URIError, AggregateError, AbstractError, - ErrorRPCRemote, - ErrorRPC, }; + /** - * Creates a replacer function that omits a specific key during serialization. - * @returns {Function} The replacer function. + * The replacer function to customize the serialization process. */ -const createReplacer = () => { - return (keyToRemove) => { - return (key, value) => { - if (key === keyToRemove) { - return undefined; +const filterSensitive = (keyToRemove) => { + return (key, value) => { + if (key === keyToRemove) { + return undefined; + } + + if (key !== 'code') { + if (value instanceof rpcErrors.ErrorRPCProtocol) { + return { + code: value.code, + message: value.message, + data: value.data, + type: value.constructor.name, + }; } - if (key !== 'code') { - if (value instanceof rpcErrors.ErrorRPCProtocol) { - return { - code: value.code, + if (value instanceof AggregateError) { + return { + type: value.constructor.name, + data: { + errors: value.errors, message: value.message, - data: value.data, - type: value.constructor.name, - }; - } - - if (value instanceof AggregateError) { - return { - type: value.constructor.name, - data: { - errors: value.errors, - message: value.message, - stack: value.stack, - }, - }; - } + stack: value.stack, + }, + }; } + } - return value; - }; + return value; }; }; -/** - * The replacer function to customize the serialization process. - */ -const filterSensitive = createReplacer(); -const ErrorCodeToErrorType: { - [code: number]: new (...args: any[]) => ErrorRPC; -} = { +const enum JSONRPCErrorCode { + ParseError = -32700, + InvalidRequest = -32600, + MethodNotFound = -32601, + InvalidParams = -32602, + InternalError = -32603, + HandlerNotFound = -32000, + RPCStopping = -32001, + RPCMessageLength = -32003, + RPCMissingResponse = -32004, + RPCOutputStreamError = -32005, + RPCRemote = -32006, + RPCStreamEnded = -32007, + RPCTimedOut = -32008, + RPCConnectionLocal = -32010, + RPCConnectionPeer = -32011, + RPCConnectionKeepAliveTimeOut = -32012, + RPCConnectionInternal = -32013, + MissingHeader = -32014, + HandlerAborted = -32015, + MissingCaller = -32016, +} + +const rpcProtocolErrors = { [JSONRPCErrorCode.RPCRemote]: ErrorRPCRemote, [JSONRPCErrorCode.RPCStopping]: ErrorRPCStopping, [JSONRPCErrorCode.RPCMessageLength]: ErrorRPCMessageLength, [JSONRPCErrorCode.ParseError]: ErrorRPCParse, - [JSONRPCErrorCode.InvalidParams]: ErrorRPC, + [JSONRPCErrorCode.InvalidParams]: ErrorRPCInvalidParams, [JSONRPCErrorCode.HandlerNotFound]: ErrorRPCHandlerFailed, [JSONRPCErrorCode.RPCMissingResponse]: ErrorRPCMissingResponse, [JSONRPCErrorCode.RPCOutputStreamError]: ErrorRPCOutputStreamError, @@ -334,57 +375,70 @@ const ErrorCodeToErrorType: { [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, [JSONRPCErrorCode.MissingCaller]: ErrorMissingCaller, }; + /** * Deserializes an error response object into an ErrorRPCRemote instance. * @param {any} errorResponse - The error response object. - * @param {any} [metadata] - Optional metadata for the deserialized error. * @returns {ErrorRPCRemote} The deserialized ErrorRPCRemote instance. * @throws {TypeError} If the errorResponse object is invalid. */ -function toError(errorData: any, clientMetadata?: any): ErrorRPC { - // Parsing if it's a string - if (typeof errorData === 'string') { +function toError(errorData: JSONValue, metadata: JSONValue): any { + // If the value is an error then reconstruct it + if ( + errorData != null && + typeof errorData === 'object' && + 'type' in errorData && + typeof errorData.type === 'string' + ) { try { - errorData = JSON.parse(errorData); + let eClass = standardErrors[errorData.type]; + if (eClass != null) { + let e: Error; + switch (eClass) { + case AbstractError: + e = eClass.fromJSON(errorData); + break; + case AggregateError: + if ( + !Array.isArray(errorData.errors) || + typeof errorData.message !== 'string' || + ('stack' in errorData && typeof errorData.stack !== 'string') + ) { + throw new TypeError(`cannot decode JSON to ${errorData.type}`); + } + e = new eClass(errorData.errors.map(toError), errorData.message); + e.stack = errorData.stack as string; + break; + default: + if ( + typeof errorData.message !== 'string' || + ('stack' in errorData && typeof errorData.stack !== 'string') + ) { + throw new TypeError(`Cannot decode JSON to ${errorData.type}`); + } + e = new (eClass as typeof Error)(errorData.message); + e.stack = errorData.stack as string; + break; + } + if ((e as any).data == null) { + (e as any).data = {}; + } + Object.assign((e as any).data, metadata); + Object.assign((e as any).data, errorData); + return e; + } } catch (e) { - throw new ErrorRPCConnectionInternal('Unable to parse string to JSON'); + // If `TypeError` which represents decoding failure + // then return value as-is + // Any other exception is a bug + if (!(e instanceof TypeError)) { + throw e; + } } } - - // Check if errorData is an object and not null - if (typeof errorData !== 'object' || errorData === null) { - throw new ErrorRPCConnectionInternal( - 'errorData should be a non-null object', - ); - } - - // Define default error values, you can modify this as per your needs - let errorCode = -32006; - let message = 'Unknown error'; - let data = {}; - - // Check for errorCode and update if exists - if ('errorCode' in errorData) { - errorCode = errorData.errorCode; - } - - if ('message' in errorData) { - message = errorData.message; - } - - if ('data' in errorData) { - data = errorData.data; - } - - // Map errorCode to a specific Error type - const ErrorType = ErrorCodeToErrorType[errorCode]; - if (!ErrorType) { - throw new ErrorRPC('Unknown Error Code'); // Handle unknown error codes - } - - const error = new ErrorType(message, { data, metadata: clientMetadata }); - return error; + // Other values are returned as-is + return errorData; } /** @@ -422,7 +476,7 @@ function clientInputTransformStream( * @param timer - Timer that gets refreshed each time a message is provided. */ function clientOutputTransformStream( - clientMetadata?: JSONValue, + clientMetadata: JSONValue, timer?: Timer, ): TransformStream, O> { return new TransformStream, O>({ @@ -430,7 +484,12 @@ function clientOutputTransformStream( timer?.refresh(); // `error` indicates it's an error message if ('error' in chunk) { - throw toError(chunk.error, clientMetadata); + if (chunk.error.code === JSONRPCErrorCode.RPCRemote) { + throw toError(JSON.parse(chunk.error.data as string), clientMetadata); + } + const e = ErrorRPCProtocol.fromJSON(chunk.error); + Object.assign(e.data, clientMetadata); + throw e; } controller.enqueue(chunk.result); }, @@ -535,6 +594,8 @@ export { getHandlerTypes, parseHeadStream, promise, + JSONRPCErrorCode, + rpcProtocolErrors, isObject, sleep, never, diff --git a/tests/RPC.test.ts b/tests/RPC.test.ts index 9cee06a..71cc4c0 100644 --- a/tests/RPC.test.ts +++ b/tests/RPC.test.ts @@ -187,7 +187,7 @@ describe('RPC', () => { rpcClient.methods.testMethod({ hello: 'world', }), - ).rejects.toThrow(rpcErrors.ErrorRPCRemote); + ).rejects.toHaveProperty('message', 'some error'); await rpcServer.stop({ force: true }); }); @@ -466,70 +466,16 @@ describe('RPC', () => { // The promise should be rejected const rejection = await callProm; + // console.log(rejection) + // The error should have specific properties - expect(rejection).toBeInstanceOf(rpcErrors.ErrorRPCRemote); - expect(rejection).toMatchObject({ code: -32006 }); + expect(rejection).toBeInstanceOf(error.constructor); + expect(rejection).toMatchObject(error); // Cleanup await rpcServer.stop({ force: true }); }, ); - - testProp( - 'RPC handles and sends sensitive errors', - [ - rpcTestUtils.safeJsonValueArb, - rpcTestUtils.errorArb(rpcTestUtils.errorArb()), - ], - async (value, error) => { - const { clientPair, serverPair } = rpcTestUtils.createTapPairs< - Uint8Array, - Uint8Array - >(); - - class TestMethod extends UnaryHandler { - public handle = async ( - _input: JSONValue, - _cancel: (reason?: any) => void, - _meta: Record | undefined, - _ctx: ContextTimed, - ): Promise => { - throw error; - }; - } - - const rpcServer = new RPCServer({ - logger, - idGen, - }); - await rpcServer.start({ - manifest: { - testMethod: new TestMethod({}), - }, - }); - rpcServer.handleStream({ ...serverPair, cancel: () => {} }); - - const rpcClient = new RPCClient({ - manifest: { - testMethod: new UnaryCaller(), - }, - streamFactory: async () => { - return { ...clientPair, cancel: () => {} }; - }, - logger, - idGen, - }); - - const callProm = rpcClient.methods.testMethod(ErrorRPCRemote.description); - - // Use Jest's `.rejects` to handle the promise rejection - await expect(callProm).rejects.toBeInstanceOf(rpcErrors.ErrorRPCRemote); - await expect(callProm).rejects.not.toHaveProperty('cause.stack'); - - await rpcServer.stop({ force: true }); - }, - ); - test('middleware can end stream early', async () => { const { clientPair, serverPair } = rpcTestUtils.createTapPairs< Uint8Array, @@ -928,12 +874,11 @@ describe('RPC', () => { ); testProp( - 'RPC Serializes and Deserializes ErrorRPCRemote', + 'RPC Serializes and Deserializes Error', [ - rpcTestUtils.safeJsonValueArb, rpcTestUtils.errorArb(rpcTestUtils.errorArb()), ], - async (value, error) => { + async (error) => { const { clientPair, serverPair } = rpcTestUtils.createTapPairs< Uint8Array, Uint8Array @@ -971,34 +916,18 @@ describe('RPC', () => { idGen, }); - const errorInstance = new ErrorRPCRemote({ - metadata: -123123, - message: 'parse error', - options: { cause: 'Random cause' }, - }); - - const serializedError = fromError(errorInstance); - const callProm = rpcClient.methods.testMethod(serializedError); - await expect(callProm).rejects.toThrow(rpcErrors.ErrorRPCRemote); - - const deserializedError = toError(serializedError); - - expect(deserializedError).toBeInstanceOf(ErrorRPCRemote); - - // Check properties explicitly - const { code, message, data } = deserializedError as ErrorRPCRemote; - expect(code).toBe(-32006); + const callProm = rpcClient.methods.testMethod({}); + const callError = await callProm.catch((e) => e); await rpcServer.stop({ force: true }); }, ); testProp( - 'RPC Serializes and Deserializes ErrorRPCRemote with Custom Replacer Function', + 'RPC Serializes and Deserializes Error with Custom Replacer Function', [ - rpcTestUtils.safeJsonValueArb, rpcTestUtils.errorArb(rpcTestUtils.errorArb()), ], - async (value, error) => { + async (error) => { const { clientPair, serverPair } = rpcTestUtils.createTapPairs< Uint8Array, Uint8Array @@ -1017,6 +946,7 @@ describe('RPC', () => { const rpcServer = new RPCServer({ logger, idGen, + replacer: filterSensitive('stack'), }); await rpcServer.start({ manifest: { @@ -1036,30 +966,8 @@ describe('RPC', () => { idGen, }); - const errorInstance = new ErrorRPCRemote({ - metadata: -32006, - message: '', - options: { - cause: error, - data: 'asda', - }, - }); - - const serializedError = JSON.parse( - JSON.stringify(fromError(errorInstance), filterSensitive('data')), - ); - - const callProm = rpcClient.methods.testMethod(serializedError); - const catchError = await callProm.catch((e) => e); - - const deserializedError = toError(serializedError); - - expect(deserializedError).toBeInstanceOf(ErrorRPCRemote); - - // Check properties explicitly - const { code, message, data } = deserializedError as ErrorRPCRemote; - expect(code).toBe(-32006); - expect(data).toBe(undefined); + const callProm = rpcClient.methods.testMethod({}); + const callError = await callProm.catch((e) => e); await rpcServer.stop({ force: true }); }, @@ -1070,7 +978,9 @@ describe('RPC', () => { Uint8Array >(); - const testReason = Error('test error'); + const errorMessage = 'test error'; + + const testReason = Error(errorMessage); class TestMethod extends UnaryHandler { public handle = async ( @@ -1115,6 +1025,6 @@ describe('RPC', () => { await rpcServer.stop({ force: true, reason: testReason }); - await expect(testProm).toReject(); + await expect(testProm).rejects.toHaveProperty('message', errorMessage); }); }); diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 51ce2c6..3a20f8c 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -23,4 +23,12 @@ describe('utils tests', () => { }, { numRuns: 1000 }, ); + test('fromError', () => { + expect(rpcUtils.fromError(undefined)).toBe(undefined); + expect(rpcUtils.fromError(null)).toBe(null); + expect(rpcUtils.fromError("123")).toBe("123"); + expect(rpcUtils.fromError(123)).toBe(123); + expect(rpcUtils.fromError(new Error("message"))).toHaveProperty("message", "message"); + expect(rpcUtils.fromError(new Error("message", { cause: new Error() }))).toHaveProperty(["cause", "type"], "Error"); + }); }); diff --git a/tests/utils.ts b/tests/utils.ts index b10c37d..10d393a 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -16,6 +16,7 @@ import * as utils from '@/utils'; import { fromError } from '@/utils'; import * as rpcErrors from '@/errors'; import { ErrorRPC } from '@/errors'; +import { AbstractError } from '@matrixai/errors'; /** * This is used to convert regular chunks into randomly sized chunks based on @@ -260,21 +261,17 @@ const errorArb = ( ) => cause.chain((cause) => fc.oneof( - fc.constant(new rpcErrors.ErrorRPCRemote()), fc.constant(new rpcErrors.ErrorRPCMessageLength(undefined)), fc.constant( - new rpcErrors.ErrorRPCRemote( - { + new AbstractError("message", { + cause, + data: { command: 'someCommand', host: `someHost`, port: 0, - }, - undefined, - { - cause, - }, - ), - ), + } + }) + ) ), ); From 7184205c3c3076c76821c8ce731d5454dfe35970 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Mon, 9 Oct 2023 18:18:35 +1100 Subject: [PATCH 2/8] fix: fixed weird use before initialization bug Import cycles, really confusing if you've never seen it before. [ci skip] --- src/errors.ts | 48 +++++++++++++++++- src/utils.ts | 134 +++++++++++++------------------------------------- 2 files changed, 80 insertions(+), 102 deletions(-) diff --git a/src/errors.ts b/src/errors.ts index 9c51885..39a6357 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,7 +1,6 @@ import type { Class } from '@matrixai/errors'; -import type { JSONRPCError, JSONValue } from '@/types'; +import type { JSONRPCError } from '@/types'; import { AbstractError } from '@matrixai/errors'; -import { JSONRPCErrorCode, rpcProtocolErrors } from './utils'; class ErrorRPC extends AbstractError { static description = 'RPC Error'; @@ -159,6 +158,50 @@ class ErrorRPCConnectionInternal extends ErrorRPCProtocol { code = JSONRPCErrorCode.RPCConnectionInternal; } +const enum JSONRPCErrorCode { + ParseError = -32700, + InvalidRequest = -32600, + MethodNotFound = -32601, + InvalidParams = -32602, + InternalError = -32603, + HandlerNotFound = -32000, + RPCStopping = -32001, + RPCMessageLength = -32003, + RPCMissingResponse = -32004, + RPCOutputStreamError = -32005, + RPCRemote = -32006, + RPCStreamEnded = -32007, + RPCTimedOut = -32008, + RPCConnectionLocal = -32010, + RPCConnectionPeer = -32011, + RPCConnectionKeepAliveTimeOut = -32012, + RPCConnectionInternal = -32013, + MissingHeader = -32014, + HandlerAborted = -32015, + MissingCaller = -32016, +} + +const rpcProtocolErrors = { + [JSONRPCErrorCode.RPCRemote]: ErrorRPCRemote, + [JSONRPCErrorCode.RPCStopping]: ErrorRPCStopping, + [JSONRPCErrorCode.RPCMessageLength]: ErrorRPCMessageLength, + [JSONRPCErrorCode.ParseError]: ErrorRPCParse, + [JSONRPCErrorCode.InvalidParams]: ErrorRPCInvalidParams, + [JSONRPCErrorCode.HandlerNotFound]: ErrorRPCHandlerFailed, + [JSONRPCErrorCode.RPCMissingResponse]: ErrorRPCMissingResponse, + [JSONRPCErrorCode.RPCOutputStreamError]: ErrorRPCOutputStreamError, + [JSONRPCErrorCode.RPCTimedOut]: ErrorRPCTimedOut, + [JSONRPCErrorCode.RPCStreamEnded]: ErrorRPCStreamEnded, + [JSONRPCErrorCode.RPCConnectionLocal]: ErrorRPCConnectionLocal, + [JSONRPCErrorCode.RPCConnectionPeer]: ErrorRPCConnectionPeer, + [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: + ErrorRPCConnectionKeepAliveTimeOut, + [JSONRPCErrorCode.RPCConnectionInternal]: ErrorRPCConnectionInternal, + [JSONRPCErrorCode.MissingHeader]: ErrorMissingHeader, + [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, + [JSONRPCErrorCode.MissingCaller]: ErrorMissingCaller, +}; + export { ErrorRPC, ErrorRPCServer, @@ -185,4 +228,5 @@ export { ErrorRPCCallerFailed, ErrorMissingCaller, JSONRPCErrorCode, + rpcProtocolErrors, }; diff --git a/src/utils.ts b/src/utils.ts index 75068a8..4ac729a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,27 +16,7 @@ import type { import { TransformStream } from 'stream/web'; import { JSONParser } from '@streamparser/json'; import { AbstractError } from '@matrixai/errors'; -import { - ErrorRPCRemote, - ErrorRPC, - ErrorRPCConnectionInternal, - ErrorRPCStopping, - ErrorRPCMessageLength, - ErrorRPCParse, - ErrorRPCHandlerFailed, - ErrorRPCMissingResponse, - ErrorRPCOutputStreamError, - ErrorRPCTimedOut, - ErrorRPCStreamEnded, - ErrorRPCConnectionLocal, - ErrorRPCConnectionPeer, - ErrorRPCConnectionKeepAliveTimeOut, - ErrorMissingHeader, - ErrorMissingCaller, - ErrorRPCProtocol, - ErrorRPCInvalidParams, -} from './errors'; -import * as rpcErrors from './errors'; +import * as errors from './errors'; // Importing PK funcs and utils which are essential for RPC function isObject(o: unknown): o is object { @@ -63,13 +43,13 @@ function parseJSONRPCRequest( message: unknown, ): JSONRPCRequest { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } if (!('method' in message)) { - throw new rpcErrors.ErrorRPCParse('`method` property must be defined'); + throw new errors.ErrorRPCParse('`method` property must be defined'); } if (typeof message.method !== 'string') { - throw new rpcErrors.ErrorRPCParse('`method` property must be a string'); + throw new errors.ErrorRPCParse('`method` property must be a string'); } // If ('params' in message && !utils.isObject(message.params)) { // throw new rpcErrors.ErrorRPCParse('`params` property must be a POJO'); @@ -82,14 +62,14 @@ function parseJSONRPCRequestMessage( ): JSONRPCRequestMessage { const jsonRequest = parseJSONRPCRequest(message); if (!('id' in jsonRequest)) { - throw new rpcErrors.ErrorRPCParse('`id` property must be defined'); + throw new errors.ErrorRPCParse('`id` property must be defined'); } if ( typeof jsonRequest.id !== 'string' && typeof jsonRequest.id !== 'number' && jsonRequest.id !== null ) { - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( '`id` property must be a string, number or null', ); } @@ -101,7 +81,7 @@ function parseJSONRPCRequestNotification( ): JSONRPCRequestNotification { const jsonRequest = parseJSONRPCRequest(message); if ('id' in jsonRequest) { - throw new rpcErrors.ErrorRPCParse('`id` property must not be defined'); + throw new errors.ErrorRPCParse('`id` property must not be defined'); } return jsonRequest as JSONRPCRequestNotification; } @@ -110,26 +90,26 @@ function parseJSONRPCResponseResult( message: unknown, ): JSONRPCResponseResult { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } if (!('result' in message)) { - throw new rpcErrors.ErrorRPCParse('`result` property must be defined'); + throw new errors.ErrorRPCParse('`result` property must be defined'); } if ('error' in message) { - throw new rpcErrors.ErrorRPCParse('`error` property must not be defined'); + throw new errors.ErrorRPCParse('`error` property must not be defined'); } // If (!utils.isObject(message.result)) { // throw new rpcErrors.ErrorRPCParse('`result` property must be a POJO'); // } if (!('id' in message)) { - throw new rpcErrors.ErrorRPCParse('`id` property must be defined'); + throw new errors.ErrorRPCParse('`id` property must be defined'); } if ( typeof message.id !== 'string' && typeof message.id !== 'number' && message.id !== null ) { - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( '`id` property must be a string, number or null', ); } @@ -138,24 +118,24 @@ function parseJSONRPCResponseResult( function parseJSONRPCResponseError(message: unknown): JSONRPCResponseError { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } if ('result' in message) { - throw new rpcErrors.ErrorRPCParse('`result` property must not be defined'); + throw new errors.ErrorRPCParse('`result` property must not be defined'); } if (!('error' in message)) { - throw new rpcErrors.ErrorRPCParse('`error` property must be defined'); + throw new errors.ErrorRPCParse('`error` property must be defined'); } parseJSONRPCError(message.error); if (!('id' in message)) { - throw new rpcErrors.ErrorRPCParse('`id` property must be defined'); + throw new errors.ErrorRPCParse('`id` property must be defined'); } if ( typeof message.id !== 'string' && typeof message.id !== 'number' && message.id !== null ) { - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( '`id` property must be a string, number or null', ); } @@ -164,19 +144,19 @@ function parseJSONRPCResponseError(message: unknown): JSONRPCResponseError { function parseJSONRPCError(message: unknown): JSONRPCError { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } if (!('code' in message)) { - throw new rpcErrors.ErrorRPCParse('`code` property must be defined'); + throw new errors.ErrorRPCParse('`code` property must be defined'); } if (typeof message.code !== 'number') { - throw new rpcErrors.ErrorRPCParse('`code` property must be a number'); + throw new errors.ErrorRPCParse('`code` property must be a number'); } if (!('message' in message)) { - throw new rpcErrors.ErrorRPCParse('`message` property must be defined'); + throw new errors.ErrorRPCParse('`message` property must be defined'); } if (typeof message.message !== 'string') { - throw new rpcErrors.ErrorRPCParse('`message` property must be a string'); + throw new errors.ErrorRPCParse('`message` property must be a string'); } // If ('data' in message && !utils.isObject(message.data)) { // throw new rpcErrors.ErrorRPCParse('`data` property must be a POJO'); @@ -188,7 +168,7 @@ function parseJSONRPCResponse( message: unknown, ): JSONRPCResponse { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } try { return parseJSONRPCResponseResult(message); @@ -200,7 +180,7 @@ function parseJSONRPCResponse( } catch (e) { // Do nothing } - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( 'structure did not match a `JSONRPCResponse`', ); } @@ -209,13 +189,13 @@ function parseJSONRPCMessage( message: unknown, ): JSONRPCMessage { if (!isObject(message)) { - throw new rpcErrors.ErrorRPCParse('must be a JSON POJO'); + throw new errors.ErrorRPCParse('must be a JSON POJO'); } if (!('jsonrpc' in message)) { - throw new rpcErrors.ErrorRPCParse('`jsonrpc` property must be defined'); + throw new errors.ErrorRPCParse('`jsonrpc` property must be defined'); } if (message.jsonrpc !== '2.0') { - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( '`jsonrpc` property must be a string of "2.0"', ); } @@ -229,7 +209,7 @@ function parseJSONRPCMessage( } catch { // Do nothing } - throw new rpcErrors.ErrorRPCParse( + throw new errors.ErrorRPCParse( 'Message structure did not match a `JSONRPCMessage`', ); } @@ -307,7 +287,7 @@ const filterSensitive = (keyToRemove) => { } if (key !== 'code') { - if (value instanceof rpcErrors.ErrorRPCProtocol) { + if (value instanceof errors.ErrorRPCProtocol) { return { code: value.code, message: value.message, @@ -332,50 +312,6 @@ const filterSensitive = (keyToRemove) => { }; }; -const enum JSONRPCErrorCode { - ParseError = -32700, - InvalidRequest = -32600, - MethodNotFound = -32601, - InvalidParams = -32602, - InternalError = -32603, - HandlerNotFound = -32000, - RPCStopping = -32001, - RPCMessageLength = -32003, - RPCMissingResponse = -32004, - RPCOutputStreamError = -32005, - RPCRemote = -32006, - RPCStreamEnded = -32007, - RPCTimedOut = -32008, - RPCConnectionLocal = -32010, - RPCConnectionPeer = -32011, - RPCConnectionKeepAliveTimeOut = -32012, - RPCConnectionInternal = -32013, - MissingHeader = -32014, - HandlerAborted = -32015, - MissingCaller = -32016, -} - -const rpcProtocolErrors = { - [JSONRPCErrorCode.RPCRemote]: ErrorRPCRemote, - [JSONRPCErrorCode.RPCStopping]: ErrorRPCStopping, - [JSONRPCErrorCode.RPCMessageLength]: ErrorRPCMessageLength, - [JSONRPCErrorCode.ParseError]: ErrorRPCParse, - [JSONRPCErrorCode.InvalidParams]: ErrorRPCInvalidParams, - [JSONRPCErrorCode.HandlerNotFound]: ErrorRPCHandlerFailed, - [JSONRPCErrorCode.RPCMissingResponse]: ErrorRPCMissingResponse, - [JSONRPCErrorCode.RPCOutputStreamError]: ErrorRPCOutputStreamError, - [JSONRPCErrorCode.RPCTimedOut]: ErrorRPCTimedOut, - [JSONRPCErrorCode.RPCStreamEnded]: ErrorRPCStreamEnded, - [JSONRPCErrorCode.RPCConnectionLocal]: ErrorRPCConnectionLocal, - [JSONRPCErrorCode.RPCConnectionPeer]: ErrorRPCConnectionPeer, - [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: - ErrorRPCConnectionKeepAliveTimeOut, - [JSONRPCErrorCode.RPCConnectionInternal]: ErrorRPCConnectionInternal, - [JSONRPCErrorCode.MissingHeader]: ErrorMissingHeader, - [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, - [JSONRPCErrorCode.MissingCaller]: ErrorMissingCaller, -}; - /** * Deserializes an error response object into an ErrorRPCRemote instance. * @param {any} errorResponse - The error response object. @@ -484,10 +420,10 @@ function clientOutputTransformStream( timer?.refresh(); // `error` indicates it's an error message if ('error' in chunk) { - if (chunk.error.code === JSONRPCErrorCode.RPCRemote) { + if (chunk.error.code === errors.JSONRPCErrorCode.RPCRemote) { throw toError(JSON.parse(chunk.error.data as string), clientMetadata); } - const e = ErrorRPCProtocol.fromJSON(chunk.error); + const e = errors.ErrorRPCProtocol.fromJSON(chunk.error); Object.assign(e.data, clientMetadata); throw e; } @@ -551,12 +487,12 @@ function parseHeadStream( bytesWritten += chunk.byteLength; parser.write(chunk); } catch (e) { - throw new rpcErrors.ErrorRPCParse(undefined, { + throw new errors.ErrorRPCParse(undefined, { cause: e, }); } if (bytesWritten > bufferByteLimit) { - throw new rpcErrors.ErrorRPCMessageLength(); + throw new errors.ErrorRPCMessageLength(); } } else { // Wait for parser to end @@ -575,7 +511,7 @@ function parseHeadStream( } function never(): never { - throw new ErrorRPC('This function should never be called'); + throw new errors.ErrorRPC('This function should never be called'); } export { @@ -594,8 +530,6 @@ export { getHandlerTypes, parseHeadStream, promise, - JSONRPCErrorCode, - rpcProtocolErrors, isObject, sleep, never, From d07e6c17d726a3a1da0aa0fac2f68dda33d0f2a8 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 19:24:14 +1100 Subject: [PATCH 3/8] feat: changed error object data structure to support metadata and timestamps --- src/RPCClient.ts | 14 ++++-- src/RPCServer.ts | 15 +++---- src/errors.ts | 70 +++++++---------------------- src/types.ts | 3 +- src/utils.ts | 106 ++++++++++++++++++++++++++++++++++---------- tests/RPC.test.ts | 14 +++--- tests/utils.test.ts | 8 ---- tests/utils.ts | 6 ++- 8 files changed, 129 insertions(+), 107 deletions(-) diff --git a/src/RPCClient.ts b/src/RPCClient.ts index b237848..db96ec2 100644 --- a/src/RPCClient.ts +++ b/src/RPCClient.ts @@ -15,7 +15,6 @@ import type { JSONRPCResponseResult, ToError, } from './types'; -import { JSONRPCErrorCode } from './errors'; import Logger from '@matrixai/logger'; import { Timer } from '@matrixai/timer'; import * as middleware from './middleware'; @@ -470,10 +469,17 @@ class RPCClient { ...(rpcStream.meta ?? {}), command: method, }; - if (messageValue.error.code === JSONRPCErrorCode.RPCRemote) { - throw this.toError(JSON.parse(messageValue.error.data as string), metadata); + const e: errors.ErrorRPCProtocol = errors.ErrorRPCProtocol.fromJSON(messageValue.error); + if ( + e instanceof errors.ErrorRPCRemote && + messageValue.error.data != null && + typeof messageValue.error.data === "object" && + 'cause' in messageValue.error.data + ) { + e.metadata = metadata; + e.cause = this.toError(JSON.parse(messageValue.error.data.cause as string)); } - throw errors.ErrorRPCProtocol.fromJSON(messageValue.error); + throw e; } leadingMessage = messageValue; } catch (e) { diff --git a/src/RPCServer.ts b/src/RPCServer.ts index 42bee73..71bd7d6 100644 --- a/src/RPCServer.ts +++ b/src/RPCServer.ts @@ -34,6 +34,7 @@ import * as utils from './utils'; import * as errors from './errors'; import * as middleware from './middleware'; import * as events from './events'; +import { POJO } from '@matrixai/errors'; const cleanupReason = Symbol('CleanupReason'); @@ -325,11 +326,8 @@ class RPCServer { rpcError = e.toJSON(); } else { - rpcError = { - code: errors.JSONRPCErrorCode.RPCRemote, - message: e?.message ?? '', - data: JSON.stringify(this.fromError(e), this.replacer), - }; + rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); + (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', @@ -588,11 +586,8 @@ class RPCServer { rpcError = e.toJSON(); } else { - rpcError = { - code: errors.JSONRPCErrorCode.RPCRemote, - message: e?.message ?? '', - data: JSON.stringify(this.fromError(e), this.replacer), - }; + rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); + (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', diff --git a/src/errors.ts b/src/errors.ts index 39a6357..1d77682 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,6 +1,7 @@ -import type { Class } from '@matrixai/errors'; -import type { JSONRPCError } from '@/types'; +import type { Class, POJO } from '@matrixai/errors'; +import type { JSONRPCError, JSONValue } from '@/types'; import { AbstractError } from '@matrixai/errors'; +import { JSONRPCErrorCode, rpcProtocolErrors } from './utils'; class ErrorRPC extends AbstractError { static description = 'RPC Error'; @@ -37,7 +38,8 @@ abstract class ErrorRPCProtocol extends ErrorRPC { if ( typeof json !== 'object' || typeof json.code !== 'number' || - typeof json.message !== 'string' + typeof json.message !== 'string' || + typeof json.data !== 'object' ) { throw new TypeError(`Cannot decode JSON to ${this.name}`); } @@ -49,19 +51,26 @@ abstract class ErrorRPCProtocol extends ErrorRPC { } const e: InstanceType = new errorC(json.message); - e.data = json.data; - e.timestamp = e.data.timestamp; + + e.stack = json.data.stack; + e.data = json.data.data; + e.timestamp = new Date(json.data.timestamp); return e; } + /** + * The return type WILL NOT include cause, this will be handled by `fromError` + * @returns + */ public toJSON(): JSONRPCError { return { code: this.code, message: this.message, data: { timestamp: this.timestamp.toJSON(), - ...this.data - } + data: this.data, + stack: this.stack, + }, } } } @@ -114,6 +123,7 @@ class ErrorRPCOutputStreamError extends ErrorRPCProtocol { class ErrorRPCRemote extends ErrorRPCProtocol { static description = 'Remote error from RPC call'; static message: string = 'The server responded with an error'; + metadata: JSONValue = {}; code = JSONRPCErrorCode.RPCRemote; } @@ -158,50 +168,6 @@ class ErrorRPCConnectionInternal extends ErrorRPCProtocol { code = JSONRPCErrorCode.RPCConnectionInternal; } -const enum JSONRPCErrorCode { - ParseError = -32700, - InvalidRequest = -32600, - MethodNotFound = -32601, - InvalidParams = -32602, - InternalError = -32603, - HandlerNotFound = -32000, - RPCStopping = -32001, - RPCMessageLength = -32003, - RPCMissingResponse = -32004, - RPCOutputStreamError = -32005, - RPCRemote = -32006, - RPCStreamEnded = -32007, - RPCTimedOut = -32008, - RPCConnectionLocal = -32010, - RPCConnectionPeer = -32011, - RPCConnectionKeepAliveTimeOut = -32012, - RPCConnectionInternal = -32013, - MissingHeader = -32014, - HandlerAborted = -32015, - MissingCaller = -32016, -} - -const rpcProtocolErrors = { - [JSONRPCErrorCode.RPCRemote]: ErrorRPCRemote, - [JSONRPCErrorCode.RPCStopping]: ErrorRPCStopping, - [JSONRPCErrorCode.RPCMessageLength]: ErrorRPCMessageLength, - [JSONRPCErrorCode.ParseError]: ErrorRPCParse, - [JSONRPCErrorCode.InvalidParams]: ErrorRPCInvalidParams, - [JSONRPCErrorCode.HandlerNotFound]: ErrorRPCHandlerFailed, - [JSONRPCErrorCode.RPCMissingResponse]: ErrorRPCMissingResponse, - [JSONRPCErrorCode.RPCOutputStreamError]: ErrorRPCOutputStreamError, - [JSONRPCErrorCode.RPCTimedOut]: ErrorRPCTimedOut, - [JSONRPCErrorCode.RPCStreamEnded]: ErrorRPCStreamEnded, - [JSONRPCErrorCode.RPCConnectionLocal]: ErrorRPCConnectionLocal, - [JSONRPCErrorCode.RPCConnectionPeer]: ErrorRPCConnectionPeer, - [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: - ErrorRPCConnectionKeepAliveTimeOut, - [JSONRPCErrorCode.RPCConnectionInternal]: ErrorRPCConnectionInternal, - [JSONRPCErrorCode.MissingHeader]: ErrorMissingHeader, - [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, - [JSONRPCErrorCode.MissingCaller]: ErrorMissingCaller, -}; - export { ErrorRPC, ErrorRPCServer, @@ -227,6 +193,4 @@ export { ErrorHandlerAborted, ErrorRPCCallerFailed, ErrorMissingCaller, - JSONRPCErrorCode, - rpcProtocolErrors, }; diff --git a/src/types.ts b/src/types.ts index 8fe6324..f9581e1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,7 +7,6 @@ import type { ServerCaller } from './callers'; import type { ClientCaller } from './callers'; import type { UnaryCaller } from './callers'; import type Handler from './handlers/Handler'; -import type { ErrorRPC } from './errors'; /** * This is the type for the IdGenFunction. It is used to generate the request @@ -350,7 +349,7 @@ type HandlerTypes = T extends Handler< type FromError = (error: any) => JSONValue; -type ToError = (errorData: JSONValue, metadata: JSONValue) => any; +type ToError = (errorData: JSONValue) => any; export type { IdGen, diff --git a/src/utils.ts b/src/utils.ts index 4ac729a..d084d76 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -239,11 +239,13 @@ function fromError( // AggregateError has an `errors` property return { type: error.constructor.name, - errors: error.errors.map(fromError), message: error.message, - stack: error.stack, - timestamp, - cause + data: { + errors: error.errors.map(fromError), + stack: error.stack, + timestamp, + cause + } }; } @@ -252,9 +254,11 @@ function fromError( return { type: error.name, message: error.message, - stack: error.stack, - timestamp, - cause, + data: { + stack: error.stack, + timestamp, + cause, + } }; } @@ -312,6 +316,50 @@ const filterSensitive = (keyToRemove) => { }; }; +const enum JSONRPCErrorCode { + ParseError = -32700, + InvalidRequest = -32600, + MethodNotFound = -32601, + InvalidParams = -32602, + InternalError = -32603, + HandlerNotFound = -32000, + RPCStopping = -32001, + RPCMessageLength = -32003, + RPCMissingResponse = -32004, + RPCOutputStreamError = -32005, + RPCRemote = -32006, + RPCStreamEnded = -32007, + RPCTimedOut = -32008, + RPCConnectionLocal = -32010, + RPCConnectionPeer = -32011, + RPCConnectionKeepAliveTimeOut = -32012, + RPCConnectionInternal = -32013, + MissingHeader = -32014, + HandlerAborted = -32015, + MissingCaller = -32016, +} + +const rpcProtocolErrors = { + [JSONRPCErrorCode.RPCRemote]: errors.ErrorRPCRemote, + [JSONRPCErrorCode.RPCStopping]: errors.ErrorRPCStopping, + [JSONRPCErrorCode.RPCMessageLength]: errors.ErrorRPCMessageLength, + [JSONRPCErrorCode.ParseError]: errors.ErrorRPCParse, + [JSONRPCErrorCode.InvalidParams]: errors.ErrorRPCInvalidParams, + [JSONRPCErrorCode.HandlerNotFound]: errors.ErrorRPCHandlerFailed, + [JSONRPCErrorCode.RPCMissingResponse]: errors.ErrorRPCMissingResponse, + [JSONRPCErrorCode.RPCOutputStreamError]: errors.ErrorRPCOutputStreamError, + [JSONRPCErrorCode.RPCTimedOut]: errors.ErrorRPCTimedOut, + [JSONRPCErrorCode.RPCStreamEnded]: errors.ErrorRPCStreamEnded, + [JSONRPCErrorCode.RPCConnectionLocal]: errors.ErrorRPCConnectionLocal, + [JSONRPCErrorCode.RPCConnectionPeer]: errors.ErrorRPCConnectionPeer, + [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: + errors.ErrorRPCConnectionKeepAliveTimeOut, + [JSONRPCErrorCode.RPCConnectionInternal]: errors.ErrorRPCConnectionInternal, + [JSONRPCErrorCode.MissingHeader]: errors.ErrorMissingHeader, + [JSONRPCErrorCode.HandlerAborted]: errors.ErrorRPCHandlerFailed, + [JSONRPCErrorCode.MissingCaller]: errors.ErrorMissingCaller, +}; + /** * Deserializes an error response object into an ErrorRPCRemote instance. * @param {any} errorResponse - The error response object. @@ -319,13 +367,15 @@ const filterSensitive = (keyToRemove) => { * @throws {TypeError} If the errorResponse object is invalid. */ -function toError(errorData: JSONValue, metadata: JSONValue): any { +function toError(errorData: JSONValue): any { // If the value is an error then reconstruct it if ( errorData != null && typeof errorData === 'object' && 'type' in errorData && - typeof errorData.type === 'string' + typeof errorData.type === 'string' && + 'data' in errorData && + typeof errorData.data === 'object' ) { try { let eClass = standardErrors[errorData.type]; @@ -337,31 +387,34 @@ function toError(errorData: JSONValue, metadata: JSONValue): any { break; case AggregateError: if ( - !Array.isArray(errorData.errors) || + errorData.data == null || + !('errors' in errorData.data) || + !Array.isArray(errorData.data.errors) || typeof errorData.message !== 'string' || - ('stack' in errorData && typeof errorData.stack !== 'string') + !('stack' in errorData.data) || + typeof errorData.data.stack !== 'string' ) { throw new TypeError(`cannot decode JSON to ${errorData.type}`); } - e = new eClass(errorData.errors.map(toError), errorData.message); - e.stack = errorData.stack as string; + e = new eClass(errorData.data.errors.map(toError), errorData.message); + e.stack = errorData.data.stack; break; default: if ( + errorData.data == null || typeof errorData.message !== 'string' || - ('stack' in errorData && typeof errorData.stack !== 'string') + !('stack' in errorData.data) || + typeof errorData.data.stack !== 'string' ) { throw new TypeError(`Cannot decode JSON to ${errorData.type}`); } e = new (eClass as typeof Error)(errorData.message); - e.stack = errorData.stack as string; + e.stack = errorData.data.stack; break; } - if ((e as any).data == null) { - (e as any).data = {}; + if (errorData.data != null && 'cause' in errorData.data) { + e.cause = toError(errorData.data.cause); } - Object.assign((e as any).data, metadata); - Object.assign((e as any).data, errorData); return e; } } catch (e) { @@ -420,11 +473,16 @@ function clientOutputTransformStream( timer?.refresh(); // `error` indicates it's an error message if ('error' in chunk) { - if (chunk.error.code === errors.JSONRPCErrorCode.RPCRemote) { - throw toError(JSON.parse(chunk.error.data as string), clientMetadata); + const e: errors.ErrorRPCProtocol = errors.ErrorRPCProtocol.fromJSON(chunk.error); + if ( + e instanceof errors.ErrorRPCRemote && + chunk.error.data != null && + typeof chunk.error.data === "object" && + 'cause' in chunk.error.data + ) { + e.metadata = clientMetadata; + e.cause = toError(JSON.parse(chunk.error.data.cause as string)); } - const e = errors.ErrorRPCProtocol.fromJSON(chunk.error); - Object.assign(e.data, clientMetadata); throw e; } controller.enqueue(chunk.result); @@ -530,6 +588,8 @@ export { getHandlerTypes, parseHeadStream, promise, + JSONRPCErrorCode, + rpcProtocolErrors, isObject, sleep, never, diff --git a/tests/RPC.test.ts b/tests/RPC.test.ts index 71cc4c0..0ad0c13 100644 --- a/tests/RPC.test.ts +++ b/tests/RPC.test.ts @@ -466,11 +466,9 @@ describe('RPC', () => { // The promise should be rejected const rejection = await callProm; - // console.log(rejection) - // The error should have specific properties expect(rejection).toBeInstanceOf(error.constructor); - expect(rejection).toMatchObject(error); + expect(rejection).toEqual(error); // Cleanup await rpcServer.stop({ force: true }); @@ -919,6 +917,8 @@ describe('RPC', () => { const callProm = rpcClient.methods.testMethod({}); const callError = await callProm.catch((e) => e); + expect(callError).toEqual(error); + await rpcServer.stop({ force: true }); }, ); @@ -969,6 +969,8 @@ describe('RPC', () => { const callProm = rpcClient.methods.testMethod({}); const callError = await callProm.catch((e) => e); + expect(callError).toEqual(error); + await rpcServer.stop({ force: true }); }, ); @@ -1024,7 +1026,9 @@ describe('RPC', () => { const testProm = rpcClient.methods.testMethod({}); await rpcServer.stop({ force: true, reason: testReason }); - - await expect(testProm).rejects.toHaveProperty('message', errorMessage); + const rejection = await testProm.catch(e => e); + expect(rejection).toBeInstanceOf(ErrorRPCRemote); + expect(rejection.cause).toBeInstanceOf(Error); + expect(rejection.cause.message).toBe(errorMessage); }); }); diff --git a/tests/utils.test.ts b/tests/utils.test.ts index 3a20f8c..51ce2c6 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -23,12 +23,4 @@ describe('utils tests', () => { }, { numRuns: 1000 }, ); - test('fromError', () => { - expect(rpcUtils.fromError(undefined)).toBe(undefined); - expect(rpcUtils.fromError(null)).toBe(null); - expect(rpcUtils.fromError("123")).toBe("123"); - expect(rpcUtils.fromError(123)).toBe(123); - expect(rpcUtils.fromError(new Error("message"))).toHaveProperty("message", "message"); - expect(rpcUtils.fromError(new Error("message", { cause: new Error() }))).toHaveProperty(["cause", "type"], "Error"); - }); }); diff --git a/tests/utils.ts b/tests/utils.ts index 10d393a..18bcb60 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -151,10 +151,12 @@ const jsonRpcErrorArb = ( { code: fc.integer(), message: fc.string(), - data: error.map((e) => JSON.stringify(fromError(e))), + data: fc.record({ + cause: error.map((e) => JSON.stringify(fromError(e))) + }), }, { - requiredKeys: ['code', 'message'], + requiredKeys: ['code', 'message', 'data'], }, ) .noShrink() as fc.Arbitrary; From ac96502c5adfcfa83a3a238adb29039a38f5d41f Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 19:48:41 +1100 Subject: [PATCH 4/8] fix: static constant for RPCRemote tests --- src/errors.ts | 10 ++++++++-- tests/utils.ts | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/errors.ts b/src/errors.ts index 1d77682..a933fe0 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -41,13 +41,13 @@ abstract class ErrorRPCProtocol extends ErrorRPC { typeof json.message !== 'string' || typeof json.data !== 'object' ) { - throw new TypeError(`Cannot decode JSON to ${this.name}`); + return new ErrorRPCUnknown(`Cannot decode JSON to ${this.name}`) as InstanceType; } const errorC = rpcProtocolErrors[json.code]; if (errorC == null) { - throw new TypeError(`Cannot decode JSON to ${this.name}`); + return new ErrorRPCUnknown(`Unknown error.code found on RPC message`) as InstanceType; } const e: InstanceType = new errorC(json.message); @@ -168,6 +168,11 @@ class ErrorRPCConnectionInternal extends ErrorRPCProtocol { code = JSONRPCErrorCode.RPCConnectionInternal; } +class ErrorRPCUnknown extends ErrorRPCProtocol { + static description = 'RPC Unknown Error'; + code = 0; +} + export { ErrorRPC, ErrorRPCServer, @@ -193,4 +198,5 @@ export { ErrorHandlerAborted, ErrorRPCCallerFailed, ErrorMissingCaller, + ErrorRPCUnknown }; diff --git a/tests/utils.ts b/tests/utils.ts index 18bcb60..f9e2849 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -149,7 +149,7 @@ const jsonRpcErrorArb = ( fc .record( { - code: fc.integer(), + code: fc.constant(utils.JSONRPCErrorCode.RPCRemote), message: fc.string(), data: fc.record({ cause: error.map((e) => JSON.stringify(fromError(e))) From 82de54c7da3f5d0974be89c624380d509a8bedbd Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 19:50:35 +1100 Subject: [PATCH 5/8] fix: moved JSONRPCErrorCode enums back to errors.ts to avoid use before bug --- src/errors.ts | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- src/utils.ts | 46 ---------------------------------------------- tests/utils.ts | 2 +- 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/errors.ts b/src/errors.ts index a933fe0..78e5f64 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,7 +1,6 @@ import type { Class, POJO } from '@matrixai/errors'; import type { JSONRPCError, JSONValue } from '@/types'; import { AbstractError } from '@matrixai/errors'; -import { JSONRPCErrorCode, rpcProtocolErrors } from './utils'; class ErrorRPC extends AbstractError { static description = 'RPC Error'; @@ -173,6 +172,50 @@ class ErrorRPCUnknown extends ErrorRPCProtocol { code = 0; } +const enum JSONRPCErrorCode { + ParseError = -32700, + InvalidRequest = -32600, + MethodNotFound = -32601, + InvalidParams = -32602, + InternalError = -32603, + HandlerNotFound = -32000, + RPCStopping = -32001, + RPCMessageLength = -32003, + RPCMissingResponse = -32004, + RPCOutputStreamError = -32005, + RPCRemote = -32006, + RPCStreamEnded = -32007, + RPCTimedOut = -32008, + RPCConnectionLocal = -32010, + RPCConnectionPeer = -32011, + RPCConnectionKeepAliveTimeOut = -32012, + RPCConnectionInternal = -32013, + MissingHeader = -32014, + HandlerAborted = -32015, + MissingCaller = -32016, +} + +const rpcProtocolErrors = { + [JSONRPCErrorCode.RPCRemote]: ErrorRPCRemote, + [JSONRPCErrorCode.RPCStopping]: ErrorRPCStopping, + [JSONRPCErrorCode.RPCMessageLength]: ErrorRPCMessageLength, + [JSONRPCErrorCode.ParseError]: ErrorRPCParse, + [JSONRPCErrorCode.InvalidParams]: ErrorRPCInvalidParams, + [JSONRPCErrorCode.HandlerNotFound]: ErrorRPCHandlerFailed, + [JSONRPCErrorCode.RPCMissingResponse]: ErrorRPCMissingResponse, + [JSONRPCErrorCode.RPCOutputStreamError]: ErrorRPCOutputStreamError, + [JSONRPCErrorCode.RPCTimedOut]: ErrorRPCTimedOut, + [JSONRPCErrorCode.RPCStreamEnded]: ErrorRPCStreamEnded, + [JSONRPCErrorCode.RPCConnectionLocal]: ErrorRPCConnectionLocal, + [JSONRPCErrorCode.RPCConnectionPeer]: ErrorRPCConnectionPeer, + [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: + ErrorRPCConnectionKeepAliveTimeOut, + [JSONRPCErrorCode.RPCConnectionInternal]: ErrorRPCConnectionInternal, + [JSONRPCErrorCode.MissingHeader]: ErrorMissingHeader, + [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, + [JSONRPCErrorCode.MissingCaller]: ErrorMissingCaller, +}; + export { ErrorRPC, ErrorRPCServer, @@ -198,5 +241,7 @@ export { ErrorHandlerAborted, ErrorRPCCallerFailed, ErrorMissingCaller, - ErrorRPCUnknown + ErrorRPCUnknown, + JSONRPCErrorCode, + rpcProtocolErrors }; diff --git a/src/utils.ts b/src/utils.ts index d084d76..647f432 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -316,50 +316,6 @@ const filterSensitive = (keyToRemove) => { }; }; -const enum JSONRPCErrorCode { - ParseError = -32700, - InvalidRequest = -32600, - MethodNotFound = -32601, - InvalidParams = -32602, - InternalError = -32603, - HandlerNotFound = -32000, - RPCStopping = -32001, - RPCMessageLength = -32003, - RPCMissingResponse = -32004, - RPCOutputStreamError = -32005, - RPCRemote = -32006, - RPCStreamEnded = -32007, - RPCTimedOut = -32008, - RPCConnectionLocal = -32010, - RPCConnectionPeer = -32011, - RPCConnectionKeepAliveTimeOut = -32012, - RPCConnectionInternal = -32013, - MissingHeader = -32014, - HandlerAborted = -32015, - MissingCaller = -32016, -} - -const rpcProtocolErrors = { - [JSONRPCErrorCode.RPCRemote]: errors.ErrorRPCRemote, - [JSONRPCErrorCode.RPCStopping]: errors.ErrorRPCStopping, - [JSONRPCErrorCode.RPCMessageLength]: errors.ErrorRPCMessageLength, - [JSONRPCErrorCode.ParseError]: errors.ErrorRPCParse, - [JSONRPCErrorCode.InvalidParams]: errors.ErrorRPCInvalidParams, - [JSONRPCErrorCode.HandlerNotFound]: errors.ErrorRPCHandlerFailed, - [JSONRPCErrorCode.RPCMissingResponse]: errors.ErrorRPCMissingResponse, - [JSONRPCErrorCode.RPCOutputStreamError]: errors.ErrorRPCOutputStreamError, - [JSONRPCErrorCode.RPCTimedOut]: errors.ErrorRPCTimedOut, - [JSONRPCErrorCode.RPCStreamEnded]: errors.ErrorRPCStreamEnded, - [JSONRPCErrorCode.RPCConnectionLocal]: errors.ErrorRPCConnectionLocal, - [JSONRPCErrorCode.RPCConnectionPeer]: errors.ErrorRPCConnectionPeer, - [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: - errors.ErrorRPCConnectionKeepAliveTimeOut, - [JSONRPCErrorCode.RPCConnectionInternal]: errors.ErrorRPCConnectionInternal, - [JSONRPCErrorCode.MissingHeader]: errors.ErrorMissingHeader, - [JSONRPCErrorCode.HandlerAborted]: errors.ErrorRPCHandlerFailed, - [JSONRPCErrorCode.MissingCaller]: errors.ErrorMissingCaller, -}; - /** * Deserializes an error response object into an ErrorRPCRemote instance. * @param {any} errorResponse - The error response object. @@ -588,8 +544,6 @@ export { getHandlerTypes, parseHeadStream, promise, - JSONRPCErrorCode, - rpcProtocolErrors, isObject, sleep, never, diff --git a/tests/utils.ts b/tests/utils.ts index f9e2849..44116ed 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -149,7 +149,7 @@ const jsonRpcErrorArb = ( fc .record( { - code: fc.constant(utils.JSONRPCErrorCode.RPCRemote), + code: fc.constant(rpcErrors.JSONRPCErrorCode.RPCRemote), message: fc.string(), data: fc.record({ cause: error.map((e) => JSON.stringify(fromError(e))) From 298a5ab310773069af95579ecb45632e340e08de Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 20:20:04 +1100 Subject: [PATCH 6/8] fix: errorObject.message now reflects error.message rather than error.description, as all root level errors have static descriptions --- src/RPCServer.ts | 26 ++++++++++++++++++++++++-- tests/RPCServer.test.ts | 6 +++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/RPCServer.ts b/src/RPCServer.ts index 71bd7d6..2f03eb2 100644 --- a/src/RPCServer.ts +++ b/src/RPCServer.ts @@ -327,7 +327,18 @@ class RPCServer { } else { rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); - (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); + try { + (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); + } + catch (e) { + (rpcError.data as POJO).cause = e; + // dispatch error in the case where the thrown value could not be parsed + this.dispatchEvent( + new events.RPCErrorEvent({ + detail: e, + }), + ); + } } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', @@ -587,7 +598,18 @@ class RPCServer { } else { rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); - (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); + try { + (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); + } + catch (e) { + (rpcError.data as POJO).cause = e; + // dispatch error in the case where the thrown value could not be parsed + this.dispatchEvent( + new events.RPCErrorEvent({ + detail: e, + }), + ); + } } const rpcErrorMessage: JSONRPCResponseError = { jsonrpc: '2.0', diff --git a/tests/RPCServer.test.ts b/tests/RPCServer.test.ts index ecb6c97..7a24c16 100644 --- a/tests/RPCServer.test.ts +++ b/tests/RPCServer.test.ts @@ -465,7 +465,7 @@ describe(`${RPCServer.name}`, () => { rpcServer.handleStream(readWriteStream); const rawErrorMessage = (await outputResult)[0]!.toString(); const errorMessage = JSON.parse(rawErrorMessage); - expect(errorMessage.error.message).toEqual(error.description); + expect(errorMessage.error.message).toEqual(error.message); reject(); await expect(errorProm).toReject(); await rpcServer.stop({ force: true }); @@ -508,7 +508,7 @@ describe(`${RPCServer.name}`, () => { rpcServer.handleStream(readWriteStream); const rawErrorMessage = (await outputResult)[0]!.toString(); const errorMessage = JSON.parse(rawErrorMessage); - expect(errorMessage.error.message).toEqual(error.description); + expect(errorMessage.error.message).toEqual(error.message); reject(); await expect(errorProm).toReject(); await rpcServer.stop({ force: true }); @@ -557,7 +557,7 @@ describe(`${RPCServer.name}`, () => { await writer.write(Buffer.from(JSON.stringify(message))); } // Abort stream - const writerReason = Symbol('writerAbort'); + const writerReason = new Error('writerAbort'); await writer.abort(writerReason); // We should get an error RPC message await expect(outputResult).toResolve(); From 51aad2f9f275f482492ebb7474b0e1ab45b00a97 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 20:39:10 +1100 Subject: [PATCH 7/8] chore: lintfix --- src/RPCClient.ts | 9 +++++--- src/RPCServer.ts | 37 +++++++++++++++++++-------------- src/callers/RawCaller.ts | 1 - src/errors.ts | 21 ++++++++++--------- src/events.ts | 1 - src/handlers/ClientHandler.ts | 2 ++ src/handlers/DuplexHandler.ts | 2 ++ src/handlers/RawHandler.ts | 2 ++ src/handlers/ServerHandler.ts | 2 ++ src/handlers/UnaryHandler.ts | 2 ++ src/types.ts | 3 ++- src/utils.ts | 39 ++++++++++++++++++----------------- tests/RPC.test.ts | 10 +++------ tests/RPCClient.test.ts | 3 +-- tests/utils.ts | 17 +++++++-------- 15 files changed, 81 insertions(+), 70 deletions(-) diff --git a/src/RPCClient.ts b/src/RPCClient.ts index db96ec2..af0c0c6 100644 --- a/src/RPCClient.ts +++ b/src/RPCClient.ts @@ -469,15 +469,18 @@ class RPCClient { ...(rpcStream.meta ?? {}), command: method, }; - const e: errors.ErrorRPCProtocol = errors.ErrorRPCProtocol.fromJSON(messageValue.error); + const e: errors.ErrorRPCProtocol = + errors.ErrorRPCProtocol.fromJSON(messageValue.error); if ( e instanceof errors.ErrorRPCRemote && messageValue.error.data != null && - typeof messageValue.error.data === "object" && + typeof messageValue.error.data === 'object' && 'cause' in messageValue.error.data ) { e.metadata = metadata; - e.cause = this.toError(JSON.parse(messageValue.error.data.cause as string)); + e.cause = this.toError( + JSON.parse(messageValue.error.data.cause as string), + ); } throw e; } diff --git a/src/RPCServer.ts b/src/RPCServer.ts index 2f03eb2..b21775e 100644 --- a/src/RPCServer.ts +++ b/src/RPCServer.ts @@ -17,6 +17,7 @@ import type { MiddlewareFactory, FromError, } from './types'; +import type { POJO } from '@matrixai/errors'; import { ReadableStream, TransformStream } from 'stream/web'; import Logger from '@matrixai/logger'; import { PromiseCancellable } from '@matrixai/async-cancellable'; @@ -34,7 +35,6 @@ import * as utils from './utils'; import * as errors from './errors'; import * as middleware from './middleware'; import * as events from './events'; -import { POJO } from '@matrixai/errors'; const cleanupReason = Symbol('CleanupReason'); @@ -321,18 +321,19 @@ class RPCServer { } controller.enqueue(value); } catch (e) { - let rpcError: JSONRPCError + let rpcError: JSONRPCError; if (e instanceof errors.ErrorRPCProtocol) { rpcError = e.toJSON(); - } - else { + } else { rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); try { - (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); - } - catch (e) { + (rpcError.data as POJO).cause = JSON.stringify( + this.fromError(e), + this.replacer, + ); + } catch (e) { (rpcError.data as POJO).cause = e; - // dispatch error in the case where the thrown value could not be parsed + // Dispatch error in the case where the thrown value could not be parsed this.dispatchEvent( new events.RPCErrorEvent({ detail: e, @@ -520,7 +521,10 @@ class RPCServer { await timer.catch(() => {}); this.dispatchEvent( new events.RPCErrorEvent({ - detail: new errors.ErrorRPCOutputStreamError(), + detail: new errors.ErrorRPCOutputStreamError( + 'Stream failed waiting for header', + { cause: newErr }, + ), }), ); return; @@ -592,18 +596,19 @@ class RPCServer { { signal: abortController.signal, timer }, ); } catch (e) { - let rpcError: JSONRPCError + let rpcError: JSONRPCError; if (e instanceof errors.ErrorRPCProtocol) { rpcError = e.toJSON(); - } - else { + } else { rpcError = new errors.ErrorRPCRemote(e?.message).toJSON(); try { - (rpcError.data as POJO).cause = JSON.stringify(this.fromError(e), this.replacer); - } - catch (e) { + (rpcError.data as POJO).cause = JSON.stringify( + this.fromError(e), + this.replacer, + ); + } catch (e) { (rpcError.data as POJO).cause = e; - // dispatch error in the case where the thrown value could not be parsed + // Dispatch error in the case where the thrown value could not be parsed this.dispatchEvent( new events.RPCErrorEvent({ detail: e, diff --git a/src/callers/RawCaller.ts b/src/callers/RawCaller.ts index a4721cf..a5e3522 100644 --- a/src/callers/RawCaller.ts +++ b/src/callers/RawCaller.ts @@ -1,4 +1,3 @@ -import type { JSONValue } from '../types'; import Caller from './Caller'; class RawCaller extends Caller { public type: 'RAW' = 'RAW' as const; diff --git a/src/errors.ts b/src/errors.ts index 78e5f64..518984a 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,4 +1,4 @@ -import type { Class, POJO } from '@matrixai/errors'; +import type { Class } from '@matrixai/errors'; import type { JSONRPCError, JSONValue } from '@/types'; import { AbstractError } from '@matrixai/errors'; @@ -31,22 +31,24 @@ abstract class ErrorRPCProtocol extends ErrorRPC { static error = 'RPC Protocol Error'; code: number; - public static fromJSON>( - json: any, - ): InstanceType { + public static fromJSON>(json: any): InstanceType { if ( typeof json !== 'object' || typeof json.code !== 'number' || typeof json.message !== 'string' || typeof json.data !== 'object' ) { - return new ErrorRPCUnknown(`Cannot decode JSON to ${this.name}`) as InstanceType; + return new ErrorRPCUnknown( + `Cannot decode JSON to ${this.name}`, + ) as InstanceType; } const errorC = rpcProtocolErrors[json.code]; if (errorC == null) { - return new ErrorRPCUnknown(`Unknown error.code found on RPC message`) as InstanceType; + return new ErrorRPCUnknown( + `Unknown error.code found on RPC message`, + ) as InstanceType; } const e: InstanceType = new errorC(json.message); @@ -70,7 +72,7 @@ abstract class ErrorRPCProtocol extends ErrorRPC { data: this.data, stack: this.stack, }, - } + }; } } @@ -84,7 +86,6 @@ class ErrorRPCInvalidParams extends ErrorRPCProtocol { code = JSONRPCErrorCode.InvalidParams; } - class ErrorRPCStopping extends ErrorRPCProtocol { static description = 'RPC is stopping'; code = JSONRPCErrorCode.RPCStopping; @@ -209,7 +210,7 @@ const rpcProtocolErrors = { [JSONRPCErrorCode.RPCConnectionLocal]: ErrorRPCConnectionLocal, [JSONRPCErrorCode.RPCConnectionPeer]: ErrorRPCConnectionPeer, [JSONRPCErrorCode.RPCConnectionKeepAliveTimeOut]: - ErrorRPCConnectionKeepAliveTimeOut, + ErrorRPCConnectionKeepAliveTimeOut, [JSONRPCErrorCode.RPCConnectionInternal]: ErrorRPCConnectionInternal, [JSONRPCErrorCode.MissingHeader]: ErrorMissingHeader, [JSONRPCErrorCode.HandlerAborted]: ErrorRPCHandlerFailed, @@ -243,5 +244,5 @@ export { ErrorMissingCaller, ErrorRPCUnknown, JSONRPCErrorCode, - rpcProtocolErrors + rpcProtocolErrors, }; diff --git a/src/events.ts b/src/events.ts index ceb5abb..565bfc7 100644 --- a/src/events.ts +++ b/src/events.ts @@ -1,4 +1,3 @@ -import type RPCServer from './RPCServer'; import type { ErrorRPCConnectionLocal, ErrorRPCConnectionPeer, diff --git a/src/handlers/ClientHandler.ts b/src/handlers/ClientHandler.ts index 5f9d80e..49571a6 100644 --- a/src/handlers/ClientHandler.ts +++ b/src/handlers/ClientHandler.ts @@ -9,10 +9,12 @@ abstract class ClientHandler< Output extends JSONValue = JSONValue, > extends Handler { public async handle( + /* eslint-disable */ input: AsyncIterableIterator, cancel: (reason?: any) => void, meta: Record | undefined, ctx: ContextTimed, + /* eslint-disable */ ): Promise { throw new ErrorRPCMethodNotImplemented(); } diff --git a/src/handlers/DuplexHandler.ts b/src/handlers/DuplexHandler.ts index e392c44..c917601 100644 --- a/src/handlers/DuplexHandler.ts +++ b/src/handlers/DuplexHandler.ts @@ -14,10 +14,12 @@ abstract class DuplexHandler< * `finally` block and check the abort signal for potential errors. */ public async *handle( + /* eslint-disable */ input: AsyncIterableIterator, cancel: (reason?: any) => void, meta: Record | undefined, ctx: ContextTimed, + /* eslint-disable */ ): AsyncIterableIterator { throw new ErrorRPCMethodNotImplemented('This method must be overwrtitten.'); } diff --git a/src/handlers/RawHandler.ts b/src/handlers/RawHandler.ts index 48677ac..c331b73 100644 --- a/src/handlers/RawHandler.ts +++ b/src/handlers/RawHandler.ts @@ -8,10 +8,12 @@ abstract class RawHandler< Container extends ContainerType = ContainerType, > extends Handler { public async handle( + /* eslint-disable */ input: [JSONRPCRequest, ReadableStream], cancel: (reason?: any) => void, meta: Record | undefined, ctx: ContextTimed, + /* eslint-disable */ ): Promise<[JSONValue, ReadableStream]> { throw new ErrorRPCMethodNotImplemented('This method must be overridden'); } diff --git a/src/handlers/ServerHandler.ts b/src/handlers/ServerHandler.ts index deee7ec..5a347d2 100644 --- a/src/handlers/ServerHandler.ts +++ b/src/handlers/ServerHandler.ts @@ -9,10 +9,12 @@ abstract class ServerHandler< Output extends JSONValue = JSONValue, > extends Handler { public async *handle( + /* eslint-disable */ input: Input, cancel: (reason?: any) => void, meta: Record | undefined, ctx: ContextTimed, + /* eslint-disable */ ): AsyncIterableIterator { throw new ErrorRPCMethodNotImplemented('This method must be overridden'); } diff --git a/src/handlers/UnaryHandler.ts b/src/handlers/UnaryHandler.ts index 3a379d7..98fad8d 100644 --- a/src/handlers/UnaryHandler.ts +++ b/src/handlers/UnaryHandler.ts @@ -9,10 +9,12 @@ abstract class UnaryHandler< Output extends JSONValue = JSONValue, > extends Handler { public async handle( + /* eslint-disable */ input: Input, cancel: (reason?: any) => void, meta: Record | undefined, ctx: ContextTimed, + /* eslint-disable */ ): Promise { throw new ErrorRPCMethodNotImplemented('This method must be overridden'); } diff --git a/src/types.ts b/src/types.ts index f9581e1..cf51d18 100644 --- a/src/types.ts +++ b/src/types.ts @@ -375,10 +375,11 @@ export type { ClientManifest, HandlerType, MapCallers, + Opaque, JSONValue, POJO, PromiseDeconstructed, HandlerTypes, FromError, - ToError + ToError, }; diff --git a/src/utils.ts b/src/utils.ts index 647f432..170f50f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -180,9 +180,7 @@ function parseJSONRPCResponse( } catch (e) { // Do nothing } - throw new errors.ErrorRPCParse( - 'structure did not match a `JSONRPCResponse`', - ); + throw new errors.ErrorRPCParse('structure did not match a `JSONRPCResponse`'); } function parseJSONRPCMessage( @@ -218,14 +216,12 @@ function parseJSONRPCMessage( * @param {Error} error - The Error instance to serialize. * @returns {JSONValue} The serialized ErrorRPC instance. */ -function fromError( - error: any, -): JSONValue { +function fromError(error: any): JSONValue { // TODO: Linked-List traversal must be done iteractively rather than recusively to prevent stack overflow. switch (typeof error) { - case "symbol": - case "bigint": - case "function": + case 'symbol': + case 'bigint': + case 'function': throw TypeError(`${error} cannot be serialized`); } @@ -234,8 +230,7 @@ function fromError( const timestamp: string = ((error as any).timestamp ?? new Date()).toJSON(); if (error instanceof AbstractError) { return error.toJSON(); - } - else if (error instanceof AggregateError) { + } else if (error instanceof AggregateError) { // AggregateError has an `errors` property return { type: error.constructor.name, @@ -244,8 +239,8 @@ function fromError( errors: error.errors.map(fromError), stack: error.stack, timestamp, - cause - } + cause, + }, }; } @@ -258,7 +253,7 @@ function fromError( stack: error.stack, timestamp, cause, - } + }, }; } @@ -269,7 +264,9 @@ function fromError( * Error constructors for non-Polykey rpcErrors * Allows these rpcErrors to be reconstructed from RPC metadata */ -const standardErrors: { [key: string]: typeof Error | typeof AggregateError | typeof AbstractError } = { +const standardErrors: { + [key: string]: typeof Error | typeof AggregateError | typeof AbstractError; +} = { Error, TypeError, SyntaxError, @@ -334,7 +331,7 @@ function toError(errorData: JSONValue): any { typeof errorData.data === 'object' ) { try { - let eClass = standardErrors[errorData.type]; + const eClass = standardErrors[errorData.type]; if (eClass != null) { let e: Error; switch (eClass) { @@ -352,7 +349,10 @@ function toError(errorData: JSONValue): any { ) { throw new TypeError(`cannot decode JSON to ${errorData.type}`); } - e = new eClass(errorData.data.errors.map(toError), errorData.message); + e = new eClass( + errorData.data.errors.map(toError), + errorData.message, + ); e.stack = errorData.data.stack; break; default: @@ -429,11 +429,12 @@ function clientOutputTransformStream( timer?.refresh(); // `error` indicates it's an error message if ('error' in chunk) { - const e: errors.ErrorRPCProtocol = errors.ErrorRPCProtocol.fromJSON(chunk.error); + const e: errors.ErrorRPCProtocol = + errors.ErrorRPCProtocol.fromJSON(chunk.error); if ( e instanceof errors.ErrorRPCRemote && chunk.error.data != null && - typeof chunk.error.data === "object" && + typeof chunk.error.data === 'object' && 'cause' in chunk.error.data ) { e.metadata = clientMetadata; diff --git a/tests/RPC.test.ts b/tests/RPC.test.ts index 0ad0c13..ab6f8a0 100644 --- a/tests/RPC.test.ts +++ b/tests/RPC.test.ts @@ -873,9 +873,7 @@ describe('RPC', () => { testProp( 'RPC Serializes and Deserializes Error', - [ - rpcTestUtils.errorArb(rpcTestUtils.errorArb()), - ], + [rpcTestUtils.errorArb(rpcTestUtils.errorArb())], async (error) => { const { clientPair, serverPair } = rpcTestUtils.createTapPairs< Uint8Array, @@ -924,9 +922,7 @@ describe('RPC', () => { ); testProp( 'RPC Serializes and Deserializes Error with Custom Replacer Function', - [ - rpcTestUtils.errorArb(rpcTestUtils.errorArb()), - ], + [rpcTestUtils.errorArb(rpcTestUtils.errorArb())], async (error) => { const { clientPair, serverPair } = rpcTestUtils.createTapPairs< Uint8Array, @@ -1026,7 +1022,7 @@ describe('RPC', () => { const testProm = rpcClient.methods.testMethod({}); await rpcServer.stop({ force: true, reason: testReason }); - const rejection = await testProm.catch(e => e); + const rejection = await testProm.catch((e) => e); expect(rejection).toBeInstanceOf(ErrorRPCRemote); expect(rejection.cause).toBeInstanceOf(Error); expect(rejection.cause.message).toBe(errorMessage); diff --git a/tests/RPCClient.test.ts b/tests/RPCClient.test.ts index c55c33f..d518616 100644 --- a/tests/RPCClient.test.ts +++ b/tests/RPCClient.test.ts @@ -295,7 +295,7 @@ describe(`${RPCClient.name}`, () => { 'generic duplex caller can throw received error message with sensitive', [ fc.array(rpcTestUtils.jsonRpcResponseResultArb()), - rpcTestUtils.jsonRpcResponseErrorArb(rpcTestUtils.errorArb(), true), + rpcTestUtils.jsonRpcResponseErrorArb(rpcTestUtils.errorArb()), ], async (messages, errorMessage) => { const inputStream = rpcTestUtils.messagesToReadableStream([ @@ -336,7 +336,6 @@ describe(`${RPCClient.name}`, () => { fc.array(rpcTestUtils.jsonRpcResponseResultArb()), rpcTestUtils.jsonRpcResponseErrorArb( rpcTestUtils.errorArb(rpcTestUtils.errorArb()), - true, ), ], async (messages, errorMessage) => { diff --git a/tests/utils.ts b/tests/utils.ts index 44116ed..7f2b3cb 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -12,11 +12,11 @@ import type { } from '@/types'; import { ReadableStream, WritableStream, TransformStream } from 'stream/web'; import { fc } from '@fast-check/jest'; +import { AbstractError } from '@matrixai/errors'; import * as utils from '@/utils'; import { fromError } from '@/utils'; import * as rpcErrors from '@/errors'; import { ErrorRPC } from '@/errors'; -import { AbstractError } from '@matrixai/errors'; /** * This is used to convert regular chunks into randomly sized chunks based on @@ -152,7 +152,7 @@ const jsonRpcErrorArb = ( code: fc.constant(rpcErrors.JSONRPCErrorCode.RPCRemote), message: fc.string(), data: fc.record({ - cause: error.map((e) => JSON.stringify(fromError(e))) + cause: error.map((e) => JSON.stringify(fromError(e))), }), }, { @@ -161,10 +161,7 @@ const jsonRpcErrorArb = ( ) .noShrink() as fc.Arbitrary; -const jsonRpcResponseErrorArb = ( - error?: fc.Arbitrary>, - sensitive: boolean = false, -) => +const jsonRpcResponseErrorArb = (error?: fc.Arbitrary>) => fc .record({ jsonrpc: fc.constant('2.0'), @@ -265,15 +262,15 @@ const errorArb = ( fc.oneof( fc.constant(new rpcErrors.ErrorRPCMessageLength(undefined)), fc.constant( - new AbstractError("message", { + new AbstractError('message', { cause, data: { command: 'someCommand', host: `someHost`, port: 0, - } - }) - ) + }, + }), + ), ), ); From 001b976bbc8b3347b288e5e75f25aaaa6b24226f Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Mon, 9 Oct 2023 20:50:05 +1100 Subject: [PATCH 8/8] fix: inline documentation for fromError/toError defaults --- src/utils.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 170f50f..9e8e7e6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -215,6 +215,7 @@ function parseJSONRPCMessage( * Serializes an Error instance into a JSONValue object suitable for RPC. * @param {Error} error - The Error instance to serialize. * @returns {JSONValue} The serialized ErrorRPC instance. + * @throws {TypeError} If the error is an instance of {@link Symbol}, {@link BigInt} or {@link Function}. */ function fromError(error: any): JSONValue { // TODO: Linked-List traversal must be done iteractively rather than recusively to prevent stack overflow. @@ -315,11 +316,9 @@ const filterSensitive = (keyToRemove) => { /** * Deserializes an error response object into an ErrorRPCRemote instance. - * @param {any} errorResponse - The error response object. - * @returns {ErrorRPCRemote} The deserialized ErrorRPCRemote instance. - * @throws {TypeError} If the errorResponse object is invalid. + * @param {JSONValue} errorData - The error data object. + * @returns {any} The deserialized error. */ - function toError(errorData: JSONValue): any { // If the value is an error then reconstruct it if (