mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Fix errors with component stacks reported as warnings (#46637)
Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: https://github.com/facebook/react-native/pull/46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c
This commit is contained in:
committed by
Facebook GitHub Bot
parent
fbda60e657
commit
cbb313253f
@@ -82,9 +82,9 @@ let warningFilter: WarningFilter = function (format) {
|
||||
return {
|
||||
finalFormat: format,
|
||||
forceDialogImmediately: false,
|
||||
suppressDialog_LEGACY: true,
|
||||
suppressDialog_LEGACY: false,
|
||||
suppressCompletely: false,
|
||||
monitorEvent: 'unknown',
|
||||
monitorEvent: 'warning_unhandled',
|
||||
monitorListVersion: 0,
|
||||
monitorSampleRate: 1,
|
||||
};
|
||||
|
||||
+3
-3
@@ -87,7 +87,7 @@ describe('LogBox', () => {
|
||||
expect.stringMatching('at DoesNotUseKey'),
|
||||
]);
|
||||
expect(spy).toHaveBeenCalledWith({
|
||||
level: 'warn',
|
||||
level: 'error',
|
||||
category: expect.stringContaining(
|
||||
'Warning: Each child in a list should have a unique',
|
||||
),
|
||||
@@ -140,7 +140,7 @@ describe('LogBox', () => {
|
||||
expect.stringMatching('at FragmentWithProp'),
|
||||
]);
|
||||
expect(spy).toHaveBeenCalledWith({
|
||||
level: 'warn',
|
||||
level: 'error',
|
||||
category: expect.stringContaining('Warning: Invalid prop'),
|
||||
componentStack: expect.anything(),
|
||||
componentStackType: expect.stringMatching(/(stack|legacy)/),
|
||||
@@ -236,7 +236,7 @@ describe('LogBox', () => {
|
||||
),
|
||||
]);
|
||||
expect(spy).toHaveBeenCalledWith({
|
||||
level: 'warn',
|
||||
level: 'error',
|
||||
category: expect.stringContaining('Warning: Manual console error'),
|
||||
componentStack: expect.anything(),
|
||||
componentStackType: 'stack',
|
||||
|
||||
@@ -506,6 +506,45 @@ describe('LogBox', () => {
|
||||
'Custom: after installing for the second time',
|
||||
);
|
||||
});
|
||||
|
||||
it('registers errors with component stack as errors by default, when ExceptionManager is registered first', () => {
|
||||
jest.spyOn(LogBoxData, 'checkWarningFilter');
|
||||
jest.spyOn(LogBoxData, 'addLog');
|
||||
|
||||
ExceptionsManager.installConsoleErrorReporter();
|
||||
LogBox.install();
|
||||
|
||||
console.error(
|
||||
'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey',
|
||||
);
|
||||
|
||||
expect(LogBoxData.addLog).toBeCalledWith(
|
||||
expect.objectContaining({level: 'error'}),
|
||||
);
|
||||
expect(LogBoxData.checkWarningFilter).toBeCalledWith(
|
||||
'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey',
|
||||
);
|
||||
});
|
||||
|
||||
it('registers errors with component stack as errors by default, when ExceptionManager is registered second', () => {
|
||||
jest.spyOn(LogBoxData, 'checkWarningFilter');
|
||||
jest.spyOn(LogBoxData, 'addLog');
|
||||
|
||||
LogBox.install();
|
||||
ExceptionsManager.installConsoleErrorReporter();
|
||||
|
||||
console.error(
|
||||
'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey',
|
||||
);
|
||||
|
||||
expect(LogBoxData.addLog).toBeCalledWith(
|
||||
expect.objectContaining({level: 'error'}),
|
||||
);
|
||||
expect(LogBoxData.checkWarningFilter).toBeCalledWith(
|
||||
'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey',
|
||||
);
|
||||
});
|
||||
|
||||
it('registers errors without component stack as errors by default, when ExceptionManager is registered first', () => {
|
||||
jest.spyOn(LogBoxData, 'checkWarningFilter');
|
||||
jest.spyOn(LogBoxData, 'addException');
|
||||
|
||||
Reference in New Issue
Block a user