From 70cc036684a52e12ab4eb8a2f6baab3e9f4a4883 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Mon, 7 Oct 2024 16:05:42 -0700 Subject: [PATCH 1/2] feat: Always preserve original messages during error serialization --- jest.config.js | 8 ++++---- src/utils.test.ts | 6 +++--- src/utils.ts | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/jest.config.js b/jest.config.js index dabcda1..c073e0f 100644 --- a/jest.config.js +++ b/jest.config.js @@ -45,10 +45,10 @@ module.exports = { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 91.89, - functions: 94.59, - lines: 92.42, - statements: 92.42, + branches: 92.59, + functions: 94.73, + lines: 92.64, + statements: 92.64, }, }, diff --git a/src/utils.test.ts b/src/utils.test.ts index 9aefd4a..62f13db 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -98,7 +98,7 @@ describe('serializeError', () => { const result = serializeError(invalidError7); expect(result).toStrictEqual({ code: rpcCodes.internal, - message: getMessageFromCode(rpcCodes.internal), + message: invalidError7.message, data: { cause: { code: invalidError7.code, @@ -209,7 +209,7 @@ describe('serializeError', () => { const result = serializeError(error); expect(result).toStrictEqual({ code: errorCodes.rpc.internal, - message: getMessageFromCode(errorCodes.rpc.internal), + message: error.message, data: { cause: { message: error.message, @@ -220,7 +220,7 @@ describe('serializeError', () => { expect(JSON.parse(JSON.stringify(result))).toStrictEqual({ code: errorCodes.rpc.internal, - message: getMessageFromCode(errorCodes.rpc.internal), + message: error.message, data: { cause: { message: error.message, diff --git a/src/utils.ts b/src/utils.ts index de3e061..6e6bb95 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -145,16 +145,37 @@ function buildError( return error; } + const originalMessage = getOriginalMessage(error); + // If the error does not match the JsonRpcError type, use the fallback error, but try to include the original error as `cause`. const cause = serializeCause(error); const fallbackWithCause = { ...fallbackError, + ...(originalMessage && { message: originalMessage }), data: { cause }, }; return fallbackWithCause; } +/** + * Attempts to extract the original `message` property from an error value of uncertain shape. + * + * @param error - The error in question. + * @returns The original message, if it exists and is a non-empty string. + */ +function getOriginalMessage(error: unknown): string | undefined { + if ( + isObject(error) && + hasProperty(error, 'message') && + typeof error.message === 'string' && + error.message.length > 0 + ) { + return error.message; + } + return undefined; +} + /** * Check if the given code is a valid JSON-RPC server error code. * From cb3a6b458eecd4bed40545dc5d8a55265787d2c4 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 8 Oct 2024 12:04:49 -0700 Subject: [PATCH 2/2] feat(streams): Add shouldPreserveMessage param to serializeError --- jest.config.js | 2 +- src/utils.test.ts | 100 +++++++++++++++++++++++++++------------------- src/utils.ts | 24 +++++++---- 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/jest.config.js b/jest.config.js index c073e0f..a38080e 100644 --- a/jest.config.js +++ b/jest.config.js @@ -45,7 +45,7 @@ module.exports = { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 92.59, + branches: 92.77, functions: 94.73, lines: 92.64, statements: 92.64, diff --git a/src/utils.test.ts b/src/utils.test.ts index 62f13db..ac97e48 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -21,7 +21,7 @@ import { dataHasCause, getMessageFromCode, serializeError } from './utils'; const rpcCodes = errorCodes.rpc; describe('serializeError', () => { - it('handles invalid error: non-object', () => { + it('serializes invalid error: non-object', () => { const result = serializeError(invalidError0); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -30,7 +30,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: null', () => { + it('serializes invalid error: null', () => { const result = serializeError(invalidError5); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -39,7 +39,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: undefined', () => { + it('serializes invalid error: undefined', () => { const result = serializeError(invalidError6); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -48,7 +48,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: array', () => { + it('serializes invalid error: array', () => { const result = serializeError(invalidError1); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -57,7 +57,27 @@ describe('serializeError', () => { }); }); - it('handles invalid error: invalid code', () => { + it('serializes invalid error: array with non-JSON values', () => { + const error = ['foo', Symbol('bar'), { baz: 'qux', symbol: Symbol('') }]; + const result = serializeError(error); + expect(result).toStrictEqual({ + code: rpcCodes.internal, + message: getMessageFromCode(rpcCodes.internal), + data: { + cause: ['foo', null, { baz: 'qux' }], + }, + }); + + expect(JSON.parse(JSON.stringify(result))).toStrictEqual({ + code: rpcCodes.internal, + message: getMessageFromCode(rpcCodes.internal), + data: { + cause: ['foo', null, { baz: 'qux' }], + }, + }); + }); + + it('serializes invalid error: invalid code', () => { const result = serializeError(invalidError2); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -66,7 +86,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: valid code, undefined message', () => { + it('serializes invalid error: valid code, undefined message', () => { const result = serializeError(invalidError3); expect(result).toStrictEqual({ code: errorCodes.rpc.internal, @@ -79,7 +99,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: non-string message with data', () => { + it('serializes invalid error: non-string message with data', () => { const result = serializeError(invalidError4); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -94,7 +114,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: invalid code with string message', () => { + it('serializes invalid error: invalid code with string message', () => { const result = serializeError(invalidError7); expect(result).toStrictEqual({ code: rpcCodes.internal, @@ -109,7 +129,7 @@ describe('serializeError', () => { }); }); - it('handles invalid error: invalid code, no message, custom fallback', () => { + it('serializes invalid error: invalid code, no message, custom fallback', () => { const result = serializeError(invalidError2, { fallbackError: { code: rpcCodes.methodNotFound, message: 'foo' }, }); @@ -120,7 +140,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: code and message only', () => { + it('serializes valid error: code and message only', () => { const result = serializeError(validError0); expect(result).toStrictEqual({ code: 4001, @@ -128,7 +148,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: code, message, and data', () => { + it('serializes valid error: code, message, and data', () => { const result = serializeError(validError1); expect(result).toStrictEqual({ code: 4001, @@ -137,7 +157,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: instantiated error', () => { + it('serializes valid error: instantiated error', () => { const result = serializeError(validError2); expect(result).toStrictEqual({ code: rpcCodes.parse, @@ -145,7 +165,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: other instantiated error', () => { + it('serializes valid error: other instantiated error', () => { const result = serializeError(validError3); expect(result).toStrictEqual({ code: rpcCodes.parse, @@ -153,7 +173,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: instantiated error with custom message and data', () => { + it('serializes valid error: instantiated error with custom message and data', () => { const result = serializeError(validError4); expect(result).toStrictEqual({ code: rpcCodes.parse, @@ -162,7 +182,7 @@ describe('serializeError', () => { }); }); - it('handles valid error: message and data', () => { + it('serializes valid error: message and data', () => { const result = serializeError(Object.assign({}, validError1)); expect(result).toStrictEqual({ code: 4001, @@ -171,7 +191,7 @@ describe('serializeError', () => { }); }); - it('handles including stack: no stack present', () => { + it('serializes valid error: no stack present', () => { const result = serializeError(validError1); expect(result).toStrictEqual({ code: 4001, @@ -180,7 +200,7 @@ describe('serializeError', () => { }); }); - it('handles including stack: string stack present', () => { + it('serializes valid error: string stack present', () => { const result = serializeError( Object.assign({}, validError1, { stack: 'foo' }), ); @@ -192,7 +212,7 @@ describe('serializeError', () => { }); }); - it('handles removing stack', () => { + it('removes the stack with `shouldIncludeStack: false`', () => { const result = serializeError( Object.assign({}, validError1, { stack: 'foo' }), { shouldIncludeStack: false }, @@ -204,7 +224,25 @@ describe('serializeError', () => { }); }); - it('handles regular Error()', () => { + it('overwrites the original message with `shouldPreserveMessage: false`', () => { + const error = new Error('foo'); + const result = serializeError(error, { + shouldPreserveMessage: false, + fallbackError: validError0, + }); + expect(result).toStrictEqual({ + code: validError0.code, + message: validError0.message, + data: { + cause: { + message: error.message, + stack: error.stack, + }, + }, + }); + }); + + it('serializes invalid error: Error', () => { const error = new Error('foo'); const result = serializeError(error); expect(result).toStrictEqual({ @@ -230,7 +268,7 @@ describe('serializeError', () => { }); }); - it('handles JsonRpcError', () => { + it('serializes valid error: JsonRpcError', () => { const error = rpcErrors.invalidParams(); const result = serializeError(error); expect(result).toStrictEqual({ @@ -246,7 +284,7 @@ describe('serializeError', () => { }); }); - it('handles class that has serialize function', () => { + it('serializes error with serialize() method', () => { class MockClass { serialize() { return { code: 1, message: 'foo' }; @@ -289,26 +327,6 @@ describe('serializeError', () => { 'Must provide fallback error with integer number code and string message.', ); }); - - it('handles arrays passed as error', () => { - const error = ['foo', Symbol('bar'), { baz: 'qux', symbol: Symbol('') }]; - const result = serializeError(error); - expect(result).toStrictEqual({ - code: rpcCodes.internal, - message: getMessageFromCode(rpcCodes.internal), - data: { - cause: ['foo', null, { baz: 'qux' }], - }, - }); - - expect(JSON.parse(JSON.stringify(result))).toStrictEqual({ - code: rpcCodes.internal, - message: getMessageFromCode(rpcCodes.internal), - data: { - cause: ['foo', null, { baz: 'qux' }], - }, - }); - }); }); describe('dataHasCause', () => { diff --git a/src/utils.ts b/src/utils.ts index 6e6bb95..e5a9080 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -96,14 +96,20 @@ export function isValidCode(code: unknown): code is number { * @param error - The error to serialize. * @param options - Options bag. * @param options.fallbackError - The error to return if the given error is - * not compatible. Should be a JSON serializable value. + * not compatible. Should be a JSON-serializable value. * @param options.shouldIncludeStack - Whether to include the error's stack * on the returned object. + * @param options.shouldPreserveMessage - Whether to preserve the error's + * message if the fallback error is used. * @returns The serialized error. */ export function serializeError( error: unknown, - { fallbackError = FALLBACK_ERROR, shouldIncludeStack = true } = {}, + { + fallbackError = FALLBACK_ERROR, + shouldIncludeStack = true, + shouldPreserveMessage = true, + } = {}, ): SerializedJsonRpcError { if (!isJsonRpcError(fallbackError)) { throw new Error( @@ -111,7 +117,7 @@ export function serializeError( ); } - const serialized = buildError(error, fallbackError); + const serialized = buildError(error, fallbackError, shouldPreserveMessage); if (!shouldIncludeStack) { delete serialized.stack; @@ -121,15 +127,18 @@ export function serializeError( } /** - * Construct a JSON-serializable object given an error and a JSON serializable `fallbackError` + * Construct a JSON-serializable object given an error and a JSON-serializable `fallbackError` * * @param error - The error in question. - * @param fallbackError - A JSON serializable fallback error. - * @returns A JSON serializable error object. + * @param fallbackError - A JSON-serializable fallback error. + * @param shouldPreserveMessage - Whether to preserve the error's message if the fallback + * error is used. + * @returns A JSON-serializable error object. */ function buildError( error: unknown, fallbackError: SerializedJsonRpcError, + shouldPreserveMessage: boolean, ): SerializedJsonRpcError { // If an error specifies a `serialize` function, we call it and return the result. if ( @@ -151,7 +160,8 @@ function buildError( const cause = serializeCause(error); const fallbackWithCause = { ...fallbackError, - ...(originalMessage && { message: originalMessage }), + ...(shouldPreserveMessage && + originalMessage && { message: originalMessage }), data: { cause }, };