Skip to content

Commit 5ba0e1f

Browse files
vzaidmanfacebook-github-bot
vzaidman
authored and
facebook-github-bot
committed
Improve how throws from components are reported to the console (#52050)
Summary: Pull Request resolved: #52050 Uncaught errors are currently raising a custom error to `console.error`: * With somewhat unclear messaging. * Only the **component stack** is reported. * The top-most stack leads to the component where the throw occurred and not to the actual error being thrown. * The actual error being thrown is never logged After this change: * We print the actual error thrown * The *Owner stack* is attached (see test plan for examples) ## Changelog: [General][Breaking] Improve messaging and add error stack trace in console errors generated on throws from components. ---- This is a breaking change because someone might be monkey-patching console.errors, or just listens to them. Reviewed By: rickhanlonii Differential Revision: D75080385 fbshipit-source-id: 824f30a804a3bb836ea1be7257784e56c00077c1
1 parent 482f737 commit 5ba0e1f

File tree

2 files changed

+48
-50
lines changed

2 files changed

+48
-50
lines changed

‎packages/react-native/Libraries/Core/ExceptionsManager.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ function reportException(
105105
// we feed back into console.error, to make sure any methods that are
106106
// monkey patched on top of console.error are called when coming from
107107
// handleException
108-
console.error(data.message);
108+
console.error(e);
109109
}
110110

111111
if (__DEV__) {

‎packages/react-native/Libraries/Core/__tests__/ExceptionsManager-test.js‎

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ function runExceptionsManagerTests() {
133133
);
134134
expect(exceptionData.isFatal).toBe(false);
135135
expect(logToConsoleInReact).toBe(false);
136-
expect(console.error).toBeCalledWith(formattedMessage);
136+
expect(console.error).toBeCalledWith(error);
137137
});
138138

139139
test('does not pop frames off the stack with framesToPop', () => {
@@ -163,16 +163,10 @@ function runExceptionsManagerTests() {
163163
expect(nativeReportException).toBeCalledTimes(1);
164164
exceptionData = nativeReportException.mock.calls[0][0];
165165
}
166-
const formattedMessage =
167-
'Error: ' +
168-
error.message +
169-
'\n\n' +
170-
'This error is located at:' +
171-
capturedErrorDefaults.componentStack;
172166
expect(getLineFromFrame(exceptionData.stack[0])).toBe(
173167
"const error = new Error('Some error happened');",
174168
);
175-
expect(console.error).toBeCalledWith(formattedMessage);
169+
expect(console.error).toBeCalledWith(error);
176170
});
177171

178172
test('wraps string in an Error and sends to handleException', () => {
@@ -211,7 +205,13 @@ function runExceptionsManagerTests() {
211205
);
212206
expect(exceptionData.isFatal).toBe(false);
213207
expect(logToConsoleInReact).toBe(false);
214-
expect(console.error).toBeCalledWith(formattedMessage);
208+
expect(console.error).toBeCalledTimes(1);
209+
// $FlowFixMe[prop-missing]
210+
expect(console.error.mock.calls[0][0]).toBeInstanceOf(
211+
ExceptionsManager.SyntheticError,
212+
);
213+
// $FlowFixMe[prop-missing]
214+
expect(console.error.mock.calls[0][0].toString()).toBe(message);
215215
});
216216

217217
test('reports "Unspecified error" if error is null', () => {
@@ -249,7 +249,15 @@ function runExceptionsManagerTests() {
249249
);
250250
expect(exceptionData.isFatal).toBe(false);
251251
expect(logToConsoleInReact).toBe(false);
252-
expect(console.error).toBeCalledWith(formattedMessage);
252+
expect(console.error).toBeCalledTimes(1);
253+
// $FlowFixMe[prop-missing]
254+
expect(console.error.mock.calls[0][0]).toBeInstanceOf(
255+
ExceptionsManager.SyntheticError,
256+
);
257+
// $FlowFixMe[prop-missing]
258+
expect(console.error.mock.calls[0][0].toString()).toBe(
259+
'Unspecified error',
260+
);
253261
});
254262

255263
test('works with a frozen error object', () => {
@@ -280,7 +288,7 @@ function runExceptionsManagerTests() {
280288
// No component stack.
281289
const formattedMessage = 'Error: ' + error.message;
282290
expect(exceptionData.message).toBe(formattedMessage);
283-
expect(console.error).toBeCalledWith(formattedMessage);
291+
expect(console.error).toBeCalledWith(error);
284292
});
285293

286294
test('does not mutate the message', () => {
@@ -312,7 +320,7 @@ function runExceptionsManagerTests() {
312320
'This error is located at:' +
313321
capturedErrorDefaults.componentStack;
314322
expect(exceptionData.message).toBe(formattedMessage);
315-
expect(console.error).toBeCalledWith(formattedMessage);
323+
expect(console.error).toBeCalledWith(error);
316324
});
317325

318326
test('can safely process the same error multiple times', () => {
@@ -360,7 +368,7 @@ function runExceptionsManagerTests() {
360368
);
361369
expect(exceptionData.isFatal).toBe(false);
362370
expect(logToConsoleInReact).toBe(false);
363-
expect(console.error).toBeCalledWith(formattedMessage);
371+
expect(console.error).toBeCalledWith(error);
364372
}
365373
});
366374
});
@@ -664,7 +672,7 @@ function runExceptionsManagerTests() {
664672
// $FlowFixMe[prop-missing]
665673
expect(console.error.mock.calls[0]).toHaveLength(1);
666674
// $FlowFixMe[prop-missing]
667-
expect(console.error.mock.calls[0][0]).toBe(formattedMessage);
675+
expect(console.error.mock.calls[0][0]).toBe(error);
668676
});
669677

670678
test('handling a non-fatal Error', () => {
@@ -697,7 +705,7 @@ function runExceptionsManagerTests() {
697705
// $FlowFixMe[prop-missing]
698706
expect(console.error.mock.calls[0]).toHaveLength(1);
699707
// $FlowFixMe[prop-missing]
700-
expect(console.error.mock.calls[0][0]).toBe(formattedMessage);
708+
expect(console.error.mock.calls[0][0]).toBe(error);
701709
});
702710

703711
test('handling a thrown string', () => {
@@ -726,7 +734,11 @@ function runExceptionsManagerTests() {
726734
// $FlowFixMe[prop-missing]
727735
expect(console.error.mock.calls[0]).toHaveLength(1);
728736
// $FlowFixMe[prop-missing]
729-
expect(console.error.mock.calls[0]).toEqual([message]);
737+
expect(console.error.mock.calls[0][0]).toBeInstanceOf(
738+
ExceptionsManager.SyntheticError,
739+
);
740+
// $FlowFixMe[prop-missing]
741+
expect(console.error.mock.calls[0][0].toString()).toBe(message);
730742
});
731743

732744
test('does not pop frames off the stack with framesToPop', () => {
@@ -759,9 +771,7 @@ function runExceptionsManagerTests() {
759771
// $FlowFixMe[prop-missing]
760772
expect(console.error.mock.calls[0]).toHaveLength(1);
761773
// $FlowFixMe[prop-missing]
762-
expect(console.error.mock.calls[0]).toEqual([
763-
'Error: ' + error.message,
764-
]);
774+
expect(console.error.mock.calls[0][0]).toBe(error);
765775
});
766776

767777
test('logs fatal "warn"-type errors', () => {
@@ -783,19 +793,17 @@ function runExceptionsManagerTests() {
783793
// $FlowFixMe[prop-missing]
784794
expect(console.error.mock.calls[0]).toHaveLength(1);
785795
// $FlowFixMe[prop-missing]
786-
expect(console.error.mock.calls[0]).toEqual([
787-
'Error: ' + error.message,
788-
]);
796+
expect(console.error.mock.calls[0][0]).toBe(error);
789797
});
790798
});
791799

792800
describe('unstable_setExceptionDecorator', () => {
793-
let mockError;
801+
let mockConsoleError;
794802
beforeEach(() => {
795803
// NOTE: We initialise a fresh mock every time using spyOn, above.
796804
// We can't use `console._errorOriginal` for this, because that's a bound
797805
// (=wrapped) version of the mock and Jest does not approve.
798-
mockError = console.error;
806+
mockConsoleError = console.error;
799807
ExceptionsManager.installConsoleErrorReporter();
800808
});
801809

@@ -861,15 +869,11 @@ function runExceptionsManagerTests() {
861869
...beforeDecorator,
862870
message: 'decorated: ' + beforeDecorator.message,
863871
});
864-
expect(mockError).toBeCalledTimes(2);
872+
expect(mockConsoleError).toBeCalledTimes(2);
865873
// $FlowFixMe[prop-missing]
866-
expect(mockError.mock.calls[0][0]).toEqual(
867-
'Error: Some error happened',
868-
);
874+
expect(mockConsoleError.mock.calls[0][0]).toBe(error);
869875
// $FlowFixMe[prop-missing]
870-
expect(mockError.mock.calls[1][0]).toMatch(
871-
'decorated: Error: Some error happened',
872-
);
876+
expect(mockConsoleError.mock.calls[1][0]).toBe(error);
873877
});
874878

875879
test('clearing a decorator', () => {
@@ -893,11 +897,9 @@ function runExceptionsManagerTests() {
893897
expect(logBoxAddException).not.toBeCalled();
894898
expect(nativeReportException).toBeCalledTimes(1);
895899
}
896-
expect(mockError).toBeCalledTimes(1);
900+
expect(mockConsoleError).toBeCalledTimes(1);
897901
// $FlowFixMe[prop-missing]
898-
expect(mockError.mock.calls[0][0]).toEqual(
899-
'Error: Some error happened',
900-
);
902+
expect(mockConsoleError.mock.calls[0][0]).toBe(error);
901903
});
902904

903905
test('prevents decorator recursion from error handler', () => {
@@ -928,15 +930,13 @@ function runExceptionsManagerTests() {
928930
/decorated: .*Some error happened/,
929931
);
930932
}
931-
expect(mockError).toBeCalledTimes(2);
933+
expect(mockConsoleError).toBeCalledTimes(2);
932934
// $FlowFixMe[prop-missing]
933-
expect(mockError.mock.calls[0][0]).toMatch(
935+
expect(mockConsoleError.mock.calls[0][0]).toMatch(
934936
/Logging an error within the decorator/,
935937
);
936938
// $FlowFixMe[prop-missing]
937-
expect(mockError.mock.calls[1][0]).toMatch(
938-
/decorated: .*Some error happened/,
939-
);
939+
expect(mockConsoleError.mock.calls[1][0]).toBe(error);
940940
});
941941

942942
test('prevents decorator recursion from console.error', () => {
@@ -974,14 +974,14 @@ function runExceptionsManagerTests() {
974974
/decorated: .*Some error happened/,
975975
);
976976
}
977-
expect(mockError).toBeCalledTimes(2);
977+
expect(mockConsoleError).toBeCalledTimes(2);
978978
// console.error calls are chained without exception pre-processing, so decorator doesn't apply
979979
// $FlowFixMe[prop-missing]
980-
expect(mockError.mock.calls[0][0].toString()).toMatch(
980+
expect(mockConsoleError.mock.calls[0][0].toString()).toMatch(
981981
/Error: Some error happened/,
982982
);
983983
// $FlowFixMe[prop-missing]
984-
expect(mockError.mock.calls[1][0]).toMatch(
984+
expect(mockConsoleError.mock.calls[1][0]).toMatch(
985985
/Logging an error within the decorator/,
986986
);
987987
});
@@ -1012,11 +1012,9 @@ function runExceptionsManagerTests() {
10121012
/Error: Some error happened/,
10131013
);
10141014
}
1015-
expect(mockError).toBeCalledTimes(1);
1015+
expect(mockConsoleError).toBeCalledTimes(1);
10161016
// $FlowFixMe[prop-missing]
1017-
expect(mockError.mock.calls[0][0]).toMatch(
1018-
/Error: Some error happened/,
1019-
);
1017+
expect(mockConsoleError.mock.calls[0][0]).toBe(error);
10201018
});
10211019

10221020
test('can handle throwing decorators recursion when exception is logged', () => {
@@ -1044,9 +1042,9 @@ function runExceptionsManagerTests() {
10441042
/Error: Some error happened/,
10451043
);
10461044
}
1047-
expect(mockError).toBeCalledTimes(1);
1045+
expect(mockConsoleError).toBeCalledTimes(1);
10481046
// $FlowFixMe[prop-missing]
1049-
expect(mockError.mock.calls[0][0].toString()).toMatch(
1047+
expect(mockConsoleError.mock.calls[0][0].toString()).toMatch(
10501048
/Error: Some error happened/,
10511049
);
10521050
});

0 commit comments

Comments
 (0)