Skip to content

Commit e0797d0

Browse files
rickhanloniifacebook-github-bot
authored andcommitted
Remove console.error patch (#48783)
Summary: Pull Request resolved: #48783 ## Overview This is the final boss of the new owner stacks feature. With owner stacks, we don't need to parse message strings to find the component stack for logbox. Instead, we can access the component stack directly with `captureOwnerStack`. This means we don't need to install the LogBox console.error patch and can greatly simplify the process of handling errors and make it more reliable. To do this, we rely only on adding LogBox to the ExceptionManager: - `reactConsoleErrorHandler` -> `LogBox.addConsoleLog` - `reportException` -> `LogBox.addException` Changelog: [General][Fixed] - Remove LogBox patch, de-duplicating errors ## Benefits As a side effect, this removes a lot of duplicate errors. For example, currently if a component throws, you get 2 errors: {F1974436906} After this, there's just the one you expect: {F1974436908} ## Followups After this lands and doesn't need reverted for some reason, we can delete a ton of code from logbox for finding and detecting stacks from errors. Reviewed By: javache Differential Revision: D68380668 fbshipit-source-id: 68112f1e3992fada4d6aaffdf9bd618ce1834f7b
1 parent e51a5f0 commit e0797d0

6 files changed

Lines changed: 276 additions & 269 deletions

File tree

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

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,16 @@ function reportException(
112112
}
113113

114114
if (__DEV__) {
115-
const LogBox = require('../LogBox/LogBox').default;
116-
LogBox.addException({
117-
...data,
118-
isComponentError: !!e.isComponentError,
119-
});
115+
// reportToConsole is only true when coming from handleException,
116+
// and the error has not yet been logged. If it's false, then it was
117+
// reported to LogBox in reactConsoleErrorHandler, and we don't need to.
118+
if (reportToConsole) {
119+
const LogBox = require('../LogBox/LogBox').default;
120+
LogBox.addException({
121+
...data,
122+
isComponentError: !!e.isComponentError,
123+
});
124+
}
120125
} else if (isFatal || e.type !== 'warn') {
121126
const NativeExceptionsManager =
122127
require('./NativeExceptionsManager').default;
@@ -223,12 +228,6 @@ function reactConsoleErrorHandler(...args) {
223228
error = firstArg;
224229
} else {
225230
const stringifySafe = require('../Utilities/stringifySafe').default;
226-
if (typeof firstArg === 'string' && firstArg.startsWith('Warning: ')) {
227-
// React warnings use console.error so that a stack trace is shown, but
228-
// we don't (currently) want these to show a redbox
229-
// (Note: Logic duplicated in polyfills/console.js.)
230-
return;
231-
}
232231
const message = args
233232
.map(arg => (typeof arg === 'string' ? arg : stringifySafe(arg)))
234233
.join(' ');
@@ -243,6 +242,21 @@ function reactConsoleErrorHandler(...args) {
243242
!global.RN$handleException ||
244243
!global.RN$handleException(error, isFatal, reportToConsole)
245244
) {
245+
if (__DEV__) {
246+
// If we're not reporting to the console in reportException,
247+
// we need to report it as a console.error here.
248+
if (!reportToConsole) {
249+
require('../LogBox/LogBox').default.addConsoleLog('error', ...args);
250+
}
251+
}
252+
if (error.message.startsWith('Warning: ')) {
253+
// React warnings use console.error so that a stack trace is shown, but
254+
// we don't (currently) want these to report to native.
255+
// Note: We can probably remove this, but would be a breaking change
256+
// if the warning module is still used somewhere.
257+
// Logic duplicated in polyfills/console.js
258+
return;
259+
}
246260
reportException(
247261
/* $FlowFixMe[class-object-subtyping] added when improving typing for this
248262
* parameters */

0 commit comments

Comments
 (0)