Skip to content

Commit

Permalink
Report deep errors in JSONReporter
Browse files Browse the repository at this point in the history
Summary:
`JsonReporter` currently serialises `Error` instances using
```js
event = Object.assign(event, {
  message: event.error.message,
  stack: event.error.stack,
});
```

This places `message` and `stack` as top-level properties, alongside `error` itself, which serialises to `{}`. Presumably the intention was to nest them under `error: {}`?

This diff nests the properties under `error`, deprecates the old "flat" versions (removing them will be semver breaking), and also usefully serialise the newer standard `AggregateError.prototype.errors` and `Error.prototype.cause`, where applicable.

```
 - **[Feature]** JsonReporter: Print `message`, `stack`, `cause`, and `errors` (where present) under `error` when a reportable event has an `error` property.
 - **[Deprecation]** JsonReporter: Deprecate printing `message` and `stack` as top-level properties when a reportable event has an `error` property.
```

Reviewed By: GijsWeterings

Differential Revision: D59805164

fbshipit-source-id: 84745a788a11623016dd322a07971b56b29e8bb7
  • Loading branch information
robhogan authored and facebook-github-bot committed Jul 16, 2024
1 parent f228acc commit 95a6063
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 7 deletions.
47 changes: 40 additions & 7 deletions packages/metro/src/lib/JsonReporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,21 @@

import type {Writable} from 'stream';

export type SerializedError = {
message: string,
stack: string,
errors?: $ReadOnlyArray<SerializedError>,
cause?: SerializedError,
...
};

export type SerializedEvent<TEvent: {[string]: any, ...}> = TEvent extends {
error: Error,
...
}
? {
...Omit<TEvent, 'error'>,
message: string,
stack: string,
error: SerializedError,
...
}
: TEvent;
Expand All @@ -37,16 +44,42 @@ class JsonReporter<TEvent: {[string]: any, ...}> {
* (Perhaps we should switch in favor of plain object?)
*/
update(event: TEvent): void {
// $FlowFixMe[method-unbinding] added when improving typing for this parameters
if (Object.prototype.toString.call(event.error) === '[object Error]') {
if (event.error instanceof Error) {
const {message, stack} = event.error;
event = Object.assign(event, {
message: event.error.message,
stack: event.error.stack,
error: serializeError(event.error),
// TODO: Preexisting issue - this writes message, stack, etc. as
// top-level siblings of event.error (which was serialized to {}), whereas it was presumably
// intended to nest them _under_ error. Fix this in a breaking change.
message,
stack,
});
}

this._stream.write(JSON.stringify(event) + '\n');
}
}

function serializeError(
e: Error,
seen: Set<Error> = new Set(),
): SerializedError {
if (seen.has(e)) {
return {message: '[circular]: ' + e.message, stack: e.stack};
}
seen.add(e);
const {message, stack, cause} = e;
const serialized: SerializedError = {message, stack};
if (e instanceof AggregateError) {
serialized.errors = [...e.errors]
.map(innerError =>
innerError instanceof Error ? serializeError(innerError, seen) : null,
)
.filter(Boolean);
}
if (cause instanceof Error) {
serialized.cause = serializeError(cause, seen);
}
return serialized;
}

module.exports = JsonReporter;
53 changes: 53 additions & 0 deletions packages/metro/src/lib/__tests__/JsonReporter-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
* @oncall react_native
*/

import JsonReporter from '../JsonReporter';

describe('JsonReporter', () => {
test('prints deeply nested errors', () => {
const mockStream = {
write: jest.fn(),
};
const reporter = new JsonReporter<{type: 'some_failure', error: Error}>(
mockStream as $FlowFixMe,
);
reporter.update({
type: 'some_failure',
error: new AggregateError(
[
new Error('test error'),
new Error('test error with a cause', {cause: new Error('cause')}),
],
'test aggregate error',
),
});
expect(mockStream.write).toHaveBeenCalled();
const deserialized = JSON.parse(mockStream.write.mock.calls[0][0]);
expect(deserialized.error).toEqual({
message: 'test aggregate error',
stack: expect.stringContaining('JsonReporter-test'),
errors: [
{
message: 'test error',
stack: expect.stringContaining('JsonReporter-test'),
},
{
message: 'test error with a cause',
stack: expect.stringContaining('JsonReporter-test'),
cause: {
message: 'cause',
stack: expect.stringContaining('JsonReporter-test'),
},
},
],
});
});
});

0 comments on commit 95a6063

Please sign in to comment.