From 9f85f1e39c8da630ea12293f3d47dbfe64de9a64 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Fri, 2 Aug 2019 09:34:54 -0700 Subject: [PATCH] Type ErrorUtils (error-guard.js) Summary: * Adds Flow types to `error-guard.js` and propagates them via the `ErrorUtils` module. * Fixes some call sites to account for the stricter (correct) types. Differential Revision: D16619538 fbshipit-source-id: c006ff2736ec380763956c4b89702cf44dd4deb0 --- .flowconfig | 3 -- .flowconfig.android | 3 -- Libraries/Core/ExceptionsManager.js | 20 ++++----- Libraries/polyfills/error-guard.js | 63 +++++++++++++++++++++-------- Libraries/vendor/core/ErrorUtils.js | 4 +- 5 files changed, 59 insertions(+), 34 deletions(-) diff --git a/.flowconfig b/.flowconfig index d978d59d5a..5a6fa355b9 100644 --- a/.flowconfig +++ b/.flowconfig @@ -11,9 +11,6 @@ ; Ignore "BUCK" generated dirs /\.buckd/ -; Ignore polyfills -.*/Libraries/polyfills/.* - ; These should not be required directly ; require from fbjs/lib instead: require('fbjs/lib/warning') .*/node_modules/warning/.* diff --git a/.flowconfig.android b/.flowconfig.android index 1b7c7f2f95..2321807c12 100644 --- a/.flowconfig.android +++ b/.flowconfig.android @@ -11,9 +11,6 @@ ; Ignore "BUCK" generated dirs /\.buckd/ -; Ignore polyfills -.*/Libraries/polyfills/.* - ; These should not be required directly ; require from fbjs/lib instead: require('fbjs/lib/warning') .*/node_modules/warning/.* diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 0521548dca..1eef1d9682 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -99,16 +99,18 @@ declare var console: typeof console & { /** * Logs exceptions to the (native) console and displays them */ -function handleException(e: Error, isFatal: boolean) { - // Workaround for reporting errors caused by `throw 'some string'` - // Unfortunately there is no way to figure out the stacktrace in this - // case, so if you ended up here trying to trace an error, look for - // `throw ''` somewhere in your codebase. - if (!e.message) { - // $FlowFixMe - cannot reassign constant, explanation above - e = new SyntheticError(e); +function handleException(e: mixed, isFatal: boolean) { + let error: Error; + if (e instanceof Error) { + error = e; + } else { + // Workaround for reporting errors caused by `throw 'some string'` + // Unfortunately there is no way to figure out the stacktrace in this + // case, so if you ended up here trying to trace an error, look for + // `throw ''` somewhere in your codebase. + error = new SyntheticError(e); } - reportException(e, isFatal); + reportException(error, isFatal); } function reactConsoleErrorHandler() { diff --git a/Libraries/polyfills/error-guard.js b/Libraries/polyfills/error-guard.js index 48ccc57933..5b968d18a3 100644 --- a/Libraries/polyfills/error-guard.js +++ b/Libraries/polyfills/error-guard.js @@ -5,18 +5,24 @@ * LICENSE file in the root directory of this source tree. * * @format + * @flow strict * @polyfill - * @nolint */ let _inGuard = 0; +type ErrorHandler = (error: mixed, isFatal: boolean) => void; +type Fn = (...Args) => Return; + /** * This is the error handler that is called when we encounter an exception * when loading a module. This will report any errors encountered before * ExceptionsManager is configured. */ -let _globalHandler = function onError(e) { +let _globalHandler: ErrorHandler = function onError( + e: mixed, + isFatal: boolean, +) { throw e; }; @@ -29,21 +35,31 @@ let _globalHandler = function onError(e) { * set) globally before requiring anything. */ const ErrorUtils = { - setGlobalHandler(fun) { + setGlobalHandler(fun: ErrorHandler): void { _globalHandler = fun; }, - getGlobalHandler() { + getGlobalHandler(): ErrorHandler { return _globalHandler; }, - reportError(error) { - _globalHandler && _globalHandler(error); + reportError(error: mixed): void { + _globalHandler && _globalHandler(error, false); }, - reportFatalError(error) { + reportFatalError(error: mixed): void { + // NOTE: This has an untyped call site in Metro. _globalHandler && _globalHandler(error, true); }, - applyWithGuard(fun, context, args) { + applyWithGuard, TOut>( + fun: Fn, + context?: ?mixed, + args?: ?TArgs, + // Unused, but some code synced from www sets it to null. + unused_onError?: null, + // Some callers pass a name here, which we ignore. + unused_name?: ?string, + ): ?TOut { try { _inGuard++; + // $FlowFixMe: TODO T48204745 (1) apply(context, null) is fine. (2) array -> rest array should work return fun.apply(context, args); } catch (e) { ErrorUtils.reportError(e); @@ -52,30 +68,41 @@ const ErrorUtils = { } return null; }, - applyWithGuardIfNeeded(fun, context, args) { + applyWithGuardIfNeeded, TOut>( + fun: Fn, + context?: ?mixed, + args?: ?TArgs, + ): ?TOut { if (ErrorUtils.inGuard()) { + // $FlowFixMe: TODO T48204745 (1) apply(context, null) is fine. (2) array -> rest array should work return fun.apply(context, args); } else { ErrorUtils.applyWithGuard(fun, context, args); } return null; }, - inGuard() { - return _inGuard; + inGuard(): boolean { + return !!_inGuard; }, - guard(fun, name, context) { + guard, TOut>( + fun: Fn, + name?: ?string, + context?: ?mixed, + ): ?(...TArgs) => ?TOut { + // TODO: (moti) T48204753 Make sure this warning is never hit and remove it - types + // should be sufficient. if (typeof fun !== 'function') { console.warn('A function must be passed to ErrorUtils.guard, got ', fun); return null; } - name = name || fun.name || ''; - function guarded() { + const guardName = name ?? fun.name ?? ''; + function guarded(...args: TArgs): ?TOut { return ErrorUtils.applyWithGuard( fun, - context || this, - arguments, + context ?? this, + args, null, - name, + guardName, ); } @@ -84,3 +111,5 @@ const ErrorUtils = { }; global.ErrorUtils = ErrorUtils; + +export type ErrorUtilsT = typeof ErrorUtils; diff --git a/Libraries/vendor/core/ErrorUtils.js b/Libraries/vendor/core/ErrorUtils.js index a54b855244..0c0f140280 100644 --- a/Libraries/vendor/core/ErrorUtils.js +++ b/Libraries/vendor/core/ErrorUtils.js @@ -8,7 +8,7 @@ * @flow strict */ -/* eslint-disable strict */ +import type {ErrorUtilsT} from '../../polyfills/error-guard.js'; /** * The particular require runtime that we are using looks for a global @@ -22,4 +22,4 @@ * that use it aren't just using a global variable, so simply export the global * variable here. ErrorUtils is originally defined in a file named error-guard.js. */ -module.exports = global.ErrorUtils; +module.exports = (global.ErrorUtils: ErrorUtilsT);