From fa4f23e4e8c45b6882a323caebef294ac69cfbf2 Mon Sep 17 00:00:00 2001 From: Christoph Nakazawa Date: Fri, 8 Nov 2019 08:29:15 -0800 Subject: [PATCH] Add codeFrame to each warning and error in LogBox Summary: This diff changes the LogBox to show the code frame for the first non-collapsed stack frame. Let me know what you think about this change! Changelog: [Internal] LogBox changes Reviewed By: rickhanlonii Differential Revision: D18372456 fbshipit-source-id: ddf6d6c53ab28d11d8355f4cb1cb071a00a7366e --- .../Core/Devtools/symbolicateStackTrace.js | 19 +++++++++-- Libraries/Core/ExceptionsManager.js | 2 +- .../Core/__tests__/ExceptionsManager-test.js | 12 ++++--- Libraries/LogBox/Data/LogBoxLog.js | 32 +++++++++++++++---- Libraries/LogBox/Data/LogBoxSymbolication.js | 12 ++++--- .../LogBox/Data/__tests__/LogBoxLog-test.js | 10 +++--- .../YellowBox/Data/YellowBoxSymbolication.js | 4 ++- 7 files changed, 67 insertions(+), 24 deletions(-) diff --git a/Libraries/Core/Devtools/symbolicateStackTrace.js b/Libraries/Core/Devtools/symbolicateStackTrace.js index 1dbcd389b1d..fca52b49d36 100644 --- a/Libraries/Core/Devtools/symbolicateStackTrace.js +++ b/Libraries/Core/Devtools/symbolicateStackTrace.js @@ -19,13 +19,27 @@ let fetch; import type {StackFrame} from '../NativeExceptionsManager'; +export type CodeFrame = $ReadOnly<{| + content: string, + location: { + row: number, + column: number, + }, + fileName: string, +|}>; + +export type SymbolicatedStackTrace = $ReadOnly<{| + stack: Array, + codeFrame: ?CodeFrame, +|}>; + function isSourcedFromDisk(sourcePath: string): boolean { return !/^http/.test(sourcePath) && /[\\/]/.test(sourcePath); } async function symbolicateStackTrace( stack: Array, -): Promise> { +): Promise { // RN currently lazy loads whatwg-fetch using a custom fetch module, which, // when called for the first time, requires and re-exports 'whatwg-fetch'. // However, when a dependency of the project tries to require whatwg-fetch @@ -74,8 +88,7 @@ async function symbolicateStackTrace( method: 'POST', body: JSON.stringify({stack: stackCopy}), }); - const json = await response.json(); - return json.stack; + return await response.json(); } module.exports = symbolicateStackTrace; diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 0227547a1b6..935a45d78ee 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -116,7 +116,7 @@ function reportException(e: ExtendedError, isFatal: boolean) { } const symbolicateStackTrace = require('./Devtools/symbolicateStackTrace'); symbolicateStackTrace(stack) - .then(prettyStack => { + .then(({stack: prettyStack}) => { if (prettyStack) { NativeExceptionsManager.updateExceptionMessage( data.message, diff --git a/Libraries/Core/__tests__/ExceptionsManager-test.js b/Libraries/Core/__tests__/ExceptionsManager-test.js index 2c7c8fbd277..a34fc2d06f1 100644 --- a/Libraries/Core/__tests__/ExceptionsManager-test.js +++ b/Libraries/Core/__tests__/ExceptionsManager-test.js @@ -39,11 +39,13 @@ describe('ExceptionsManager', () => { }; }); // Make symbolication a no-op. - jest.mock('../Devtools/symbolicateStackTrace', () => { - return async function symbolicateStackTrace(stack) { - return stack; - }; - }); + jest.mock( + '../Devtools/symbolicateStackTrace', + () => + async function symbolicateStackTrace(stack) { + return {stack}; + }, + ); jest.spyOn(console, 'error').mockImplementation(() => {}); ReactFiberErrorDialog = require('../ReactFiberErrorDialog'); NativeExceptionsManager = require('../NativeExceptionsManager').default; diff --git a/Libraries/LogBox/Data/LogBoxLog.js b/Libraries/LogBox/Data/LogBoxLog.js index e53379f80dc..8b8c8cfbd15 100644 --- a/Libraries/LogBox/Data/LogBoxLog.js +++ b/Libraries/LogBox/Data/LogBoxLog.js @@ -82,13 +82,33 @@ class LogBoxLog { let aborted = false; if (this.symbolicated.status !== 'COMPLETE') { - const updateStatus = (error: ?Error, stack: ?Stack): void => { + const updateStatus = ( + error: ?Error, + stack: ?Stack, + codeFrame: ?CodeFrame, + ): void => { if (error != null) { - this.symbolicated = {error, stack: null, status: 'FAILED'}; + this.symbolicated = { + error, + stack: null, + status: 'FAILED', + }; } else if (stack != null) { - this.symbolicated = {error: null, stack, status: 'COMPLETE'}; + if (codeFrame) { + this.codeFrame = codeFrame; + } + + this.symbolicated = { + error: null, + stack, + status: 'COMPLETE', + }; } else { - this.symbolicated = {error: null, stack: null, status: 'PENDING'}; + this.symbolicated = { + error: null, + stack: null, + status: 'PENDING', + }; } if (!aborted) { if (callback != null) { @@ -99,8 +119,8 @@ class LogBoxLog { updateStatus(null, null); LogBoxSymbolication.symbolicate(this.stack).then( - stack => { - updateStatus(null, stack); + data => { + updateStatus(null, data?.stack, data?.codeFrame); }, error => { updateStatus(error, null); diff --git a/Libraries/LogBox/Data/LogBoxSymbolication.js b/Libraries/LogBox/Data/LogBoxSymbolication.js index 55d1b957392..2b3a1a8004d 100644 --- a/Libraries/LogBox/Data/LogBoxSymbolication.js +++ b/Libraries/LogBox/Data/LogBoxSymbolication.js @@ -13,15 +13,19 @@ import symbolicateStackTrace from '../../Core/Devtools/symbolicateStackTrace'; import type {StackFrame} from '../../Core/NativeExceptionsManager'; +import type {SymbolicatedStackTrace} from '../../Core/Devtools/symbolicateStackTrace'; export type Stack = Array; -const cache: Map> = new Map(); +const cache: Map> = new Map(); /** * Sanitize because sometimes, `symbolicateStackTrace` gives us invalid values. */ -const sanitize = (maybeStack: mixed): Stack => { +const sanitize = ({ + stack: maybeStack, + codeFrame, +}: SymbolicatedStackTrace): SymbolicatedStackTrace => { if (!Array.isArray(maybeStack)) { throw new Error('Expected stack to be an array.'); } @@ -58,14 +62,14 @@ const sanitize = (maybeStack: mixed): Stack => { collapse, }); } - return stack; + return {stack, codeFrame}; }; export function deleteStack(stack: Stack): void { cache.delete(stack); } -export function symbolicate(stack: Stack): Promise { +export function symbolicate(stack: Stack): Promise { let promise = cache.get(stack); if (promise == null) { promise = symbolicateStackTrace(stack).then(sanitize); diff --git a/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js index 189d27e7730..7649357be4b 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js @@ -12,6 +12,7 @@ 'use strict'; import type {StackFrame} from '../../../Core/NativeExceptionsManager'; +import type {SymbolicatedStackTrace} from '../../../Core/Devtools/symbolicateStackTrace'; jest.mock('../LogBoxSymbolication', () => { return {__esModule: true, symbolicate: jest.fn(), deleteStack: jest.fn()}; @@ -35,7 +36,7 @@ function getLogBoxLog() { function getLogBoxSymbolication(): {| symbolicate: JestMockFn< $ReadOnlyArray>, - Promise>, + Promise, >, |} { return (require('../LogBoxSymbolication'): any); @@ -53,9 +54,10 @@ describe('LogBoxLog', () => { beforeEach(() => { jest.resetModules(); - getLogBoxSymbolication().symbolicate.mockImplementation(async stack => - createStack(stack.map(frame => `S(${frame.methodName})`)), - ); + getLogBoxSymbolication().symbolicate.mockImplementation(async stack => ({ + stack: createStack(stack.map(frame => `S(${frame.methodName})`)), + codeFrame: null, + })); }); it('creates a LogBoxLog object', () => { diff --git a/Libraries/YellowBox/Data/YellowBoxSymbolication.js b/Libraries/YellowBox/Data/YellowBoxSymbolication.js index 08a089c3915..a33c3501e9b 100644 --- a/Libraries/YellowBox/Data/YellowBoxSymbolication.js +++ b/Libraries/YellowBox/Data/YellowBoxSymbolication.js @@ -13,6 +13,7 @@ const symbolicateStackTrace = require('../../Core/Devtools/symbolicateStackTrace'); import type {StackFrame} from '../../Core/NativeExceptionsManager'; +import type {SymbolicatedStackTrace} from '../../Core/Devtools/symbolicateStackTrace'; type CacheKey = string; @@ -45,7 +46,8 @@ const getCacheKey = (stack: Stack): CacheKey => { /** * Sanitize because sometimes, `symbolicateStackTrace` gives us invalid values. */ -const sanitize = (maybeStack: mixed): Stack => { +const sanitize = (data: SymbolicatedStackTrace): Stack => { + const maybeStack = data?.stack; if (!Array.isArray(maybeStack)) { throw new Error('Expected stack to be an array.'); }