From 0dba0aff185f4fd46e1146362235e68e52c59556 Mon Sep 17 00:00:00 2001
From: Tim Yung <yungsters@fb.com>
Date: Wed, 19 May 2021 12:27:30 -0700
Subject: [PATCH] ExceptionsManager: Minor Code Cleanup

Summary:
Cleans up `reactConsoleErrorHandler` in `ExceptionsManager` using modern language features, and fixes a minor edge case with how warning-like errors are handled.

Changelog:
[General][Fixed] - Avoid downgrading `console.error` when passed warning-like objects.

Reviewed By: GijsWeterings

Differential Revision: D28418488

fbshipit-source-id: 394e8608c2c81c794c9a0fc155142dcfcfe1c661
---
 Libraries/Core/ExceptionsManager.js           | 36 ++++++++++---------
 .../Core/__tests__/ExceptionsManager-test.js  | 12 +++++++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js
index a22a17daee1d24..fd1afec1cea227 100644
--- a/Libraries/Core/ExceptionsManager.js
+++ b/Libraries/Core/ExceptionsManager.js
@@ -175,9 +175,9 @@ function handleException(e: mixed, isFatal: boolean) {
   }
 }
 
-function reactConsoleErrorHandler() {
+function reactConsoleErrorHandler(...args) {
   // bubble up to any original handlers
-  console._errorOriginal.apply(console, arguments);
+  console._errorOriginal(...args);
   if (!console.reportErrorsAsExceptions) {
     return;
   }
@@ -213,31 +213,33 @@ function reactConsoleErrorHandler() {
     return;
   }
 
-  if (arguments[0] && arguments[0].stack) {
+  let error;
+
+  const firstArg = args[0];
+  if (firstArg?.stack) {
     // reportException will console.error this with high enough fidelity.
-    reportException(
-      arguments[0],
-      /* isFatal */ false,
-      /*reportToConsole*/ false,
-    );
+    error = firstArg;
   } else {
     const stringifySafe = require('../Utilities/stringifySafe').default;
-    const str = Array.prototype.map
-      .call(arguments, value =>
-        typeof value === 'string' ? value : stringifySafe(value),
-      )
-      .join(' ');
-
-    if (str.slice(0, 9) === 'Warning: ') {
+    if (typeof firstArg === 'string' && firstArg.startsWith('Warning: ')) {
       // React warnings use console.error so that a stack trace is shown, but
       // we don't (currently) want these to show a redbox
       // (Note: Logic duplicated in polyfills/console.js.)
       return;
     }
-    const error: ExtendedError = new SyntheticError(str);
+    const message = args
+      .map(arg => (typeof arg === 'string' ? arg : stringifySafe(arg)))
+      .join(' ');
+
+    error = new SyntheticError(message);
     error.name = 'console.error';
-    reportException(error, /* isFatal */ false, /*reportToConsole*/ false);
   }
+
+  reportException(
+    error,
+    false, // isFatal
+    false, // reportToConsole
+  );
 }
 
 /**
diff --git a/Libraries/Core/__tests__/ExceptionsManager-test.js b/Libraries/Core/__tests__/ExceptionsManager-test.js
index fe7e79730d303a..228bd9c2b3b8ec 100644
--- a/Libraries/Core/__tests__/ExceptionsManager-test.js
+++ b/Libraries/Core/__tests__/ExceptionsManager-test.js
@@ -358,6 +358,18 @@ describe('ExceptionsManager', () => {
       expect(mockError.mock.calls[0]).toEqual(args);
     });
 
+    test('logging a warning-looking object', () => {
+      // Forces `strignifySafe` to invoke `toString()`.
+      const object = {toString: () => 'Warning: Some error may have happened'};
+      object.cycle = object;
+
+      const args = [object];
+
+      console.error(...args);
+
+      expect(nativeReportException).toHaveBeenCalled();
+    });
+
     test('reportErrorsAsExceptions = false', () => {
       console.reportErrorsAsExceptions = false;
       const message = 'Some error happened';