Skip to content

Commit 807a177

Browse files
rekmarksrekmarks
authored
feat: Preserve original messages during error serialization by default (#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`).
1 parent 156c5c9 commit 807a177

File tree

3 files changed

+103
-54
lines changed

3 files changed

+103
-54
lines changed

‎jest.config.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ module.exports = {
4545
// An object that configures minimum threshold enforcement for coverage results
4646
coverageThreshold: {
4747
global: {
48-
branches: 91.89,
49-
functions: 94.59,
50-
lines: 92.42,
51-
statements: 92.42,
48+
branches: 92.77,
49+
functions: 94.73,
50+
lines: 92.64,
51+
statements: 92.64,
5252
},
5353
},
5454

‎src/utils.test.ts‎

Lines changed: 62 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { dataHasCause, getMessageFromCode, serializeError } from './utils';
2121
const rpcCodes = errorCodes.rpc;
2222

2323
describe('serializeError', () => {
24-
it('handles invalid error: non-object', () => {
24+
it('serializes invalid error: non-object', () => {
2525
const result = serializeError(invalidError0);
2626
expect(result).toStrictEqual({
2727
code: rpcCodes.internal,
@@ -30,7 +30,7 @@ describe('serializeError', () => {
3030
});
3131
});
3232

33-
it('handles invalid error: null', () => {
33+
it('serializes invalid error: null', () => {
3434
const result = serializeError(invalidError5);
3535
expect(result).toStrictEqual({
3636
code: rpcCodes.internal,
@@ -39,7 +39,7 @@ describe('serializeError', () => {
3939
});
4040
});
4141

42-
it('handles invalid error: undefined', () => {
42+
it('serializes invalid error: undefined', () => {
4343
const result = serializeError(invalidError6);
4444
expect(result).toStrictEqual({
4545
code: rpcCodes.internal,
@@ -48,7 +48,7 @@ describe('serializeError', () => {
4848
});
4949
});
5050

51-
it('handles invalid error: array', () => {
51+
it('serializes invalid error: array', () => {
5252
const result = serializeError(invalidError1);
5353
expect(result).toStrictEqual({
5454
code: rpcCodes.internal,
@@ -57,7 +57,27 @@ describe('serializeError', () => {
5757
});
5858
});
5959

60-
it('handles invalid error: invalid code', () => {
60+
it('serializes invalid error: array with non-JSON values', () => {
61+
const error = ['foo', Symbol('bar'), { baz: 'qux', symbol: Symbol('') }];
62+
const result = serializeError(error);
63+
expect(result).toStrictEqual({
64+
code: rpcCodes.internal,
65+
message: getMessageFromCode(rpcCodes.internal),
66+
data: {
67+
cause: ['foo', null, { baz: 'qux' }],
68+
},
69+
});
70+
71+
expect(JSON.parse(JSON.stringify(result))).toStrictEqual({
72+
code: rpcCodes.internal,
73+
message: getMessageFromCode(rpcCodes.internal),
74+
data: {
75+
cause: ['foo', null, { baz: 'qux' }],
76+
},
77+
});
78+
});
79+
80+
it('serializes invalid error: invalid code', () => {
6181
const result = serializeError(invalidError2);
6282
expect(result).toStrictEqual({
6383
code: rpcCodes.internal,
@@ -66,7 +86,7 @@ describe('serializeError', () => {
6686
});
6787
});
6888

69-
it('handles invalid error: valid code, undefined message', () => {
89+
it('serializes invalid error: valid code, undefined message', () => {
7090
const result = serializeError(invalidError3);
7191
expect(result).toStrictEqual({
7292
code: errorCodes.rpc.internal,
@@ -79,7 +99,7 @@ describe('serializeError', () => {
7999
});
80100
});
81101

82-
it('handles invalid error: non-string message with data', () => {
102+
it('serializes invalid error: non-string message with data', () => {
83103
const result = serializeError(invalidError4);
84104
expect(result).toStrictEqual({
85105
code: rpcCodes.internal,
@@ -94,11 +114,11 @@ describe('serializeError', () => {
94114
});
95115
});
96116

97-
it('handles invalid error: invalid code with string message', () => {
117+
it('serializes invalid error: invalid code with string message', () => {
98118
const result = serializeError(invalidError7);
99119
expect(result).toStrictEqual({
100120
code: rpcCodes.internal,
101-
message: getMessageFromCode(rpcCodes.internal),
121+
message: invalidError7.message,
102122
data: {
103123
cause: {
104124
code: invalidError7.code,
@@ -109,7 +129,7 @@ describe('serializeError', () => {
109129
});
110130
});
111131

112-
it('handles invalid error: invalid code, no message, custom fallback', () => {
132+
it('serializes invalid error: invalid code, no message, custom fallback', () => {
113133
const result = serializeError(invalidError2, {
114134
fallbackError: { code: rpcCodes.methodNotFound, message: 'foo' },
115135
});
@@ -120,15 +140,15 @@ describe('serializeError', () => {
120140
});
121141
});
122142

123-
it('handles valid error: code and message only', () => {
143+
it('serializes valid error: code and message only', () => {
124144
const result = serializeError(validError0);
125145
expect(result).toStrictEqual({
126146
code: 4001,
127147
message: validError0.message,
128148
});
129149
});
130150

131-
it('handles valid error: code, message, and data', () => {
151+
it('serializes valid error: code, message, and data', () => {
132152
const result = serializeError(validError1);
133153
expect(result).toStrictEqual({
134154
code: 4001,
@@ -137,23 +157,23 @@ describe('serializeError', () => {
137157
});
138158
});
139159

140-
it('handles valid error: instantiated error', () => {
160+
it('serializes valid error: instantiated error', () => {
141161
const result = serializeError(validError2);
142162
expect(result).toStrictEqual({
143163
code: rpcCodes.parse,
144164
message: getMessageFromCode(rpcCodes.parse),
145165
});
146166
});
147167

148-
it('handles valid error: other instantiated error', () => {
168+
it('serializes valid error: other instantiated error', () => {
149169
const result = serializeError(validError3);
150170
expect(result).toStrictEqual({
151171
code: rpcCodes.parse,
152172
message: dummyMessage,
153173
});
154174
});
155175

156-
it('handles valid error: instantiated error with custom message and data', () => {
176+
it('serializes valid error: instantiated error with custom message and data', () => {
157177
const result = serializeError(validError4);
158178
expect(result).toStrictEqual({
159179
code: rpcCodes.parse,
@@ -162,7 +182,7 @@ describe('serializeError', () => {
162182
});
163183
});
164184

165-
it('handles valid error: message and data', () => {
185+
it('serializes valid error: message and data', () => {
166186
const result = serializeError(Object.assign({}, validError1));
167187
expect(result).toStrictEqual({
168188
code: 4001,
@@ -171,7 +191,7 @@ describe('serializeError', () => {
171191
});
172192
});
173193

174-
it('handles including stack: no stack present', () => {
194+
it('serializes valid error: no stack present', () => {
175195
const result = serializeError(validError1);
176196
expect(result).toStrictEqual({
177197
code: 4001,
@@ -180,7 +200,7 @@ describe('serializeError', () => {
180200
});
181201
});
182202

183-
it('handles including stack: string stack present', () => {
203+
it('serializes valid error: string stack present', () => {
184204
const result = serializeError(
185205
Object.assign({}, validError1, { stack: 'foo' }),
186206
);
@@ -192,7 +212,7 @@ describe('serializeError', () => {
192212
});
193213
});
194214

195-
it('handles removing stack', () => {
215+
it('removes the stack with `shouldIncludeStack: false`', () => {
196216
const result = serializeError(
197217
Object.assign({}, validError1, { stack: 'foo' }),
198218
{ shouldIncludeStack: false },
@@ -204,12 +224,30 @@ describe('serializeError', () => {
204224
});
205225
});
206226

207-
it('handles regular Error()', () => {
227+
it('overwrites the original message with `shouldPreserveMessage: false`', () => {
228+
const error = new Error('foo');
229+
const result = serializeError(error, {
230+
shouldPreserveMessage: false,
231+
fallbackError: validError0,
232+
});
233+
expect(result).toStrictEqual({
234+
code: validError0.code,
235+
message: validError0.message,
236+
data: {
237+
cause: {
238+
message: error.message,
239+
stack: error.stack,
240+
},
241+
},
242+
});
243+
});
244+
245+
it('serializes invalid error: Error', () => {
208246
const error = new Error('foo');
209247
const result = serializeError(error);
210248
expect(result).toStrictEqual({
211249
code: errorCodes.rpc.internal,
212-
message: getMessageFromCode(errorCodes.rpc.internal),
250+
message: error.message,
213251
data: {
214252
cause: {
215253
message: error.message,
@@ -220,7 +258,7 @@ describe('serializeError', () => {
220258

221259
expect(JSON.parse(JSON.stringify(result))).toStrictEqual({
222260
code: errorCodes.rpc.internal,
223-
message: getMessageFromCode(errorCodes.rpc.internal),
261+
message: error.message,
224262
data: {
225263
cause: {
226264
message: error.message,
@@ -230,7 +268,7 @@ describe('serializeError', () => {
230268
});
231269
});
232270

233-
it('handles JsonRpcError', () => {
271+
it('serializes valid error: JsonRpcError', () => {
234272
const error = rpcErrors.invalidParams();
235273
const result = serializeError(error);
236274
expect(result).toStrictEqual({
@@ -246,7 +284,7 @@ describe('serializeError', () => {
246284
});
247285
});
248286

249-
it('handles class that has serialize function', () => {
287+
it('serializes error with serialize() method', () => {
250288
class MockClass {
251289
serialize() {
252290
return { code: 1, message: 'foo' };
@@ -289,26 +327,6 @@ describe('serializeError', () => {
289327
'Must provide fallback error with integer number code and string message.',
290328
);
291329
});
292-
293-
it('handles arrays passed as error', () => {
294-
const error = ['foo', Symbol('bar'), { baz: 'qux', symbol: Symbol('') }];
295-
const result = serializeError(error);
296-
expect(result).toStrictEqual({
297-
code: rpcCodes.internal,
298-
message: getMessageFromCode(rpcCodes.internal),
299-
data: {
300-
cause: ['foo', null, { baz: 'qux' }],
301-
},
302-
});
303-
304-
expect(JSON.parse(JSON.stringify(result))).toStrictEqual({
305-
code: rpcCodes.internal,
306-
message: getMessageFromCode(rpcCodes.internal),
307-
data: {
308-
cause: ['foo', null, { baz: 'qux' }],
309-
},
310-
});
311-
});
312330
});
313331

314332
describe('dataHasCause', () => {

‎src/utils.ts‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,22 +96,28 @@ export function isValidCode(code: unknown): code is number {
9696
* @param error - The error to serialize.
9797
* @param options - Options bag.
9898
* @param options.fallbackError - The error to return if the given error is
99-
* not compatible. Should be a JSON serializable value.
99+
* not compatible. Should be a JSON-serializable value.
100100
* @param options.shouldIncludeStack - Whether to include the error's stack
101101
* on the returned object.
102+
* @param options.shouldPreserveMessage - Whether to preserve the error's
103+
* message if the fallback error is used.
102104
* @returns The serialized error.
103105
*/
104106
export function serializeError(
105107
error: unknown,
106-
{ fallbackError = FALLBACK_ERROR, shouldIncludeStack = true } = {},
108+
{
109+
fallbackError = FALLBACK_ERROR,
110+
shouldIncludeStack = true,
111+
shouldPreserveMessage = true,
112+
} = {},
107113
): SerializedJsonRpcError {
108114
if (!isJsonRpcError(fallbackError)) {
109115
throw new Error(
110116
'Must provide fallback error with integer number code and string message.',
111117
);
112118
}
113119

114-
const serialized = buildError(error, fallbackError);
120+
const serialized = buildError(error, fallbackError, shouldPreserveMessage);
115121

116122
if (!shouldIncludeStack) {
117123
delete serialized.stack;
@@ -121,15 +127,18 @@ export function serializeError(
121127
}
122128

123129
/**
124-
* Construct a JSON-serializable object given an error and a JSON serializable `fallbackError`
130+
* Construct a JSON-serializable object given an error and a JSON-serializable `fallbackError`
125131
*
126132
* @param error - The error in question.
127-
* @param fallbackError - A JSON serializable fallback error.
128-
* @returns A JSON serializable error object.
133+
* @param fallbackError - A JSON-serializable fallback error.
134+
* @param shouldPreserveMessage - Whether to preserve the error's message if the fallback
135+
* error is used.
136+
* @returns A JSON-serializable error object.
129137
*/
130138
function buildError(
131139
error: unknown,
132140
fallbackError: SerializedJsonRpcError,
141+
shouldPreserveMessage: boolean,
133142
): SerializedJsonRpcError {
134143
// If an error specifies a `serialize` function, we call it and return the result.
135144
if (
@@ -145,16 +154,38 @@ function buildError(
145154
return error;
146155
}
147156

157+
const originalMessage = getOriginalMessage(error);
158+
148159
// If the error does not match the JsonRpcError type, use the fallback error, but try to include the original error as `cause`.
149160
const cause = serializeCause(error);
150161
const fallbackWithCause = {
151162
...fallbackError,
163+
...(shouldPreserveMessage &&
164+
originalMessage && { message: originalMessage }),
152165
data: { cause },
153166
};
154167

155168
return fallbackWithCause;
156169
}
157170

171+
/**
172+
* Attempts to extract the original `message` property from an error value of uncertain shape.
173+
*
174+
* @param error - The error in question.
175+
* @returns The original message, if it exists and is a non-empty string.
176+
*/
177+
function getOriginalMessage(error: unknown): string | undefined {
178+
if (
179+
isObject(error) &&
180+
hasProperty(error, 'message') &&
181+
typeof error.message === 'string' &&
182+
error.message.length > 0
183+
) {
184+
return error.message;
185+
}
186+
return undefined;
187+
}
188+
158189
/**
159190
* Check if the given code is a valid JSON-RPC server error code.
160191
*

0 commit comments

Comments
 (0)