Skip to content

Commit

Permalink
feat: Preserve original messages during error serialization by default (
Browse files Browse the repository at this point in the history 
#158)

This ensures that non-empty string `error.message` properties of
serialized errors are preserved by default, even if the serialized error
is not [a valid JSON-RPC
error](https://www.jsonrpc.org/specification#error_object). This
behavior can be overridden by setting `shouldPreserveMessage: false`.

In #61, our error serialization logic was considerably improved. One of
the behavioral changes made at the time was to always overwrite the
`message` property with that of the fallback error (practically always
the "internal JSON-RPC-error"), regardless of whether a non-empty string
message was present on the original error object. We have yet to ship
this everywhere in our stack, in part because such a change may be
breaking for our consumers. By reverting to the old behavior for the
`message` property only, we avoid these potential breakages and improve
the accessibility of potentially useful information to consumers (i.e.
directly in the error message as opposed to buried in
`error.data.cause.message`).
  • Loading branch information
@rekmarks
rekmarks authored Oct 8, 2024
1 parent 156c5c9 commit 807a177
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 54 deletions.
8 changes: 4 additions & 4 deletions jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.77,
functions: 94.73,
lines: 92.64,
statements: 92.64,
},
},

Expand Down
106 changes: 62 additions & 44 deletions src/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -94,11 +114,11 @@ 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,
message: getMessageFromCode(rpcCodes.internal),
message: invalidError7.message,
data: {
cause: {
code: invalidError7.code,
Expand All @@ -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' },
});
Expand All @@ -120,15 +140,15 @@ 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,
message: validError0.message,
});
});

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,
Expand All @@ -137,23 +157,23 @@ describe('serializeError', () => {
});
});

it('handles valid error: instantiated error', () => {
it('serializes valid error: instantiated error', () => {
const result = serializeError(validError2);
expect(result).toStrictEqual({
code: rpcCodes.parse,
message: getMessageFromCode(rpcCodes.parse),
});
});

it('handles valid error: other instantiated error', () => {
it('serializes valid error: other instantiated error', () => {
const result = serializeError(validError3);
expect(result).toStrictEqual({
code: rpcCodes.parse,
message: dummyMessage,
});
});

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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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' }),
);
Expand All @@ -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 },
Expand All @@ -204,12 +224,30 @@ 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({
code: errorCodes.rpc.internal,
message: getMessageFromCode(errorCodes.rpc.internal),
message: error.message,
data: {
cause: {
message: error.message,
Expand All @@ -220,7 +258,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,
Expand All @@ -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({
Expand All @@ -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' };
Expand Down Expand Up @@ -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', () => {
Expand Down
43 changes: 37 additions & 6 deletions src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,28 @@ 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(
'Must provide fallback error with integer number code and string message.',
);
}

const serialized = buildError(error, fallbackError);
const serialized = buildError(error, fallbackError, shouldPreserveMessage);

if (!shouldIncludeStack) {
delete serialized.stack;
Expand All @@ -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 (
Expand All @@ -145,16 +154,38 @@ 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,
...(shouldPreserveMessage &&
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.
*
Expand Down

0 comments on commit 807a177

Please sign in to comment.