Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: add more information to serializeError #16682

Merged
merged 2 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/connect/src/constants/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ export class TrezorError extends Error {
this.code = code;
this.message = message;
}

// Error.prototype.toString() does not include custom property `code`
toString() {
return `${this.name} (code: ${this.code}): ${this.message}`;
}
Copy link
Contributor Author

@Lemonexe Lemonexe Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Error.prototype.toString returns only ${this.name}: ${this.message}, but code is an important info for debug (see definition above in this file)

}

export const TypedError = (id: ErrorCode, message?: string) =>
Expand Down
8 changes: 6 additions & 2 deletions packages/utils/src/serializeError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
* Serialize an error of unknown type to a string.
*/
export const serializeError = (error: unknown): string => {
// Error instances are objects, but have no JSON printable properties
// Error instances are objects, but have no JSON printable properties.
// Instead, .toString() is their standard string representation. Though stack trace must be included separately
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/toString
if (error instanceof Error) {
return error.toString();
return JSON.stringify({ message: error.toString(), stackTrace: error.stack });
}

// plain javascript object is not a conventional error type, but we have to count with it
if (typeof error === 'object') {
return JSON.stringify(error);
}
Expand Down
21 changes: 18 additions & 3 deletions packages/utils/tests/serializeError.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,32 @@ class CustomError extends Error {
somethingExtra = 'extra stuff';

toString() {
return `${super.toString()} + ${this.somethingExtra} `;
return `${super.toString()} + ${this.somethingExtra}`;
}
}

describe(serializeError.name, () => {
it('serializes an Error instance', () => {
const error = new Error('example message');
expect(serializeError(error)).toBe('Error: example message');
/*
A very crude way to mock the stack trace.. But although we can generally mock anything in Jest,
(in this case that would be Error.prototype.captureStackTrace that sets the Error.stack property)
specifically errors are not mockable, as Jest relies on them internally and it bugs out Jest.
But couldn't we then do it at least for CustomError? No, because for Error instances,
JavaScript bypasses the prototype chain and calls Error.prototype.captureStackTrace directly.
*/
error.stack = 'Mock Stack Trace';
expect(JSON.parse(serializeError(error))).toMatchObject({
message: 'Error: example message',
stackTrace: 'Mock Stack Trace',
});

const customError = new CustomError('example message');
expect(serializeError(customError)).toBe('Error: example message + extra stuff ');
customError.stack = 'Mock Stack Trace';
expect(JSON.parse(serializeError(customError))).toMatchObject({
message: 'Error: example message + extra stuff',
stackTrace: 'Mock Stack Trace',
});
});

it('serializes a plain object', () => {
Expand Down
Loading