From 468d1a2d2e6c72d7c6d435ecaad8499997584de6 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 22 Oct 2019 11:03:29 -0700 Subject: [PATCH] Render collapsed frames in RedBox Summary: Renders frames in RedBox in a greyed-out style when their `collapse` field is set to `true`. This avoids outright hiding information in the stack trace while still drawing attention to frames that are likely to be more meaningful. Changelog: [General] [Changed] - Render collapsed JavaScript frames in RedBox Reviewed By: rickhanlonii Differential Revision: D18039438 fbshipit-source-id: 527588f11c0bff495842be7036cd1293bab65eb9 --- Libraries/Core/ExceptionsManager.js | 5 +--- React/Base/RCTJSStackFrame.h | 3 ++- React/Base/RCTJSStackFrame.m | 12 ++++++---- React/Modules/RCTRedBox.m | 5 +++- .../react/devsupport/RedBoxDialog.java | 2 ++ .../react/devsupport/StackTraceHelper.java | 24 +++++++++++++++---- .../devsupport/interfaces/StackFrame.java | 3 +++ 7 files changed, 40 insertions(+), 14 deletions(-) diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 583cb7cac2c..d97a4d468ac 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -106,12 +106,9 @@ function reportException(e: ExtendedError, isFatal: boolean) { symbolicateStackTrace(stack) .then(prettyStack => { if (prettyStack) { - const stackWithoutCollapsedFrames = prettyStack.filter( - frame => !frame.collapse, - ); NativeExceptionsManager.updateExceptionMessage( data.message, - stackWithoutCollapsedFrames, + prettyStack, currentExceptionID, ); } else { diff --git a/React/Base/RCTJSStackFrame.h b/React/Base/RCTJSStackFrame.h index 1badccdc4bb..73c6b834147 100644 --- a/React/Base/RCTJSStackFrame.h +++ b/React/Base/RCTJSStackFrame.h @@ -13,8 +13,9 @@ @property (nonatomic, copy, readonly) NSString *file; @property (nonatomic, readonly) NSInteger lineNumber; @property (nonatomic, readonly) NSInteger column; +@property (nonatomic, readonly) NSInteger collapse; -- (instancetype)initWithMethodName:(NSString *)methodName file:(NSString *)file lineNumber:(NSInteger)lineNumber column:(NSInteger)column; +- (instancetype)initWithMethodName:(NSString *)methodName file:(NSString *)file lineNumber:(NSInteger)lineNumber column:(NSInteger)column collapse:(NSInteger)collapse; - (NSDictionary *)toDictionary; + (instancetype)stackFrameWithLine:(NSString *)line; diff --git a/React/Base/RCTJSStackFrame.m b/React/Base/RCTJSStackFrame.m index 3701007e7fe..db177ddea08 100644 --- a/React/Base/RCTJSStackFrame.m +++ b/React/Base/RCTJSStackFrame.m @@ -53,13 +53,14 @@ static NSRegularExpression *RCTJSStackFrameRegex() @implementation RCTJSStackFrame -- (instancetype)initWithMethodName:(NSString *)methodName file:(NSString *)file lineNumber:(NSInteger)lineNumber column:(NSInteger)column +- (instancetype)initWithMethodName:(NSString *)methodName file:(NSString *)file lineNumber:(NSInteger)lineNumber column:(NSInteger)column collapse:(NSInteger)collapse { if (self = [super init]) { _methodName = methodName; _file = file; _lineNumber = lineNumber; _column = column; + _collapse = collapse; } return self; } @@ -70,7 +71,8 @@ static NSRegularExpression *RCTJSStackFrameRegex() @"methodName": RCTNullIfNil(self.methodName), @"file": RCTNullIfNil(self.file), @"lineNumber": @(self.lineNumber), - @"column": @(self.column) + @"column": @(self.column), + @"collapse": @(self.collapse) }; } @@ -91,7 +93,8 @@ static NSRegularExpression *RCTJSStackFrameRegex() return [[self alloc] initWithMethodName:methodName file:file lineNumber:[lineNumber integerValue] - column:[column integerValue]]; + column:[column integerValue] + collapse:@NO]; } + (instancetype)stackFrameWithDictionary:(NSDictionary *)dict @@ -99,7 +102,8 @@ static NSRegularExpression *RCTJSStackFrameRegex() return [[self alloc] initWithMethodName:RCTNilIfNull(dict[@"methodName"]) file:dict[@"file"] lineNumber:[RCTNilIfNull(dict[@"lineNumber"]) integerValue] - column:[RCTNilIfNull(dict[@"column"]) integerValue]]; + column:[RCTNilIfNull(dict[@"column"]) integerValue] + collapse:[RCTNilIfNull(dict[@"collapse"]) integerValue]]; } + (NSArray *)stackFramesWithLines:(NSString *)lines diff --git a/React/Modules/RCTRedBox.m b/React/Modules/RCTRedBox.m index 85db19c6ba1..ae227b25c8d 100644 --- a/React/Modules/RCTRedBox.m +++ b/React/Modules/RCTRedBox.m @@ -353,7 +353,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) { if (!cell) { cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"cell"]; - cell.textLabel.textColor = [UIColor whiteColor]; cell.textLabel.font = [UIFont fontWithName:@"Menlo-Regular" size:14]; cell.textLabel.lineBreakMode = NSLineBreakByCharWrapping; cell.textLabel.numberOfLines = 2; @@ -371,6 +370,10 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) } else { cell.detailTextLabel.text = @""; } + cell.textLabel.textColor = stackFrame.collapse ? [UIColor lightGrayColor] : [UIColor whiteColor]; + cell.detailTextLabel.textColor = stackFrame.collapse ? + [UIColor colorWithRed:0.50 green:0.50 blue:0.50 alpha:1.0] : + [UIColor colorWithRed:0.70 green:0.70 blue:0.70 alpha:1.0]; return cell; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java index 2c34638b86e..f23aa04cab5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java @@ -186,6 +186,8 @@ import org.json.JSONObject; FrameViewHolder holder = (FrameViewHolder) convertView.getTag(); holder.mMethodView.setText(frame.getMethod()); holder.mFileView.setText(StackTraceHelper.formatFrameSource(frame)); + holder.mMethodView.setTextColor(frame.isCollapsed() ? 0xFFAAAAAA : Color.WHITE); + holder.mFileView.setTextColor(frame.isCollapsed() ? 0xFF808080 : 0xFFB3B3B3); return convertView; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java index 11eff365ab0..053a6e8dd92 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java @@ -38,13 +38,19 @@ public class StackTraceHelper { private final int mLine; private final int mColumn; private final String mFileName; + private final boolean mIsCollapsed; - private StackFrameImpl(String file, String method, int line, int column) { + private StackFrameImpl(String file, String method, int line, int column, boolean isCollapsed) { mFile = file; mMethod = method; mLine = line; mColumn = column; mFileName = file != null ? new File(file).getName() : ""; + mIsCollapsed = isCollapsed; + } + + private StackFrameImpl(String file, String method, int line, int column) { + this(file, method, line, column, false); } private StackFrameImpl(String file, String fileName, String method, int line, int column) { @@ -53,6 +59,7 @@ public class StackTraceHelper { mMethod = method; mLine = line; mColumn = column; + mIsCollapsed = false; } /** @@ -90,6 +97,10 @@ public class StackTraceHelper { return mFileName; } + public boolean isCollapsed() { + return mIsCollapsed; + } + /** Convert the stack frame to a JSON representation. */ public JSONObject toJSON() { return new JSONObject( @@ -97,7 +108,8 @@ public class StackTraceHelper { "file", getFile(), "methodName", getMethod(), "lineNumber", getLine(), - "column", getColumn())); + "column", getColumn(), + "collapse", isCollapsed())); } } @@ -114,6 +126,8 @@ public class StackTraceHelper { ReadableMap frame = stack.getMap(i); String methodName = frame.getString("methodName"); String fileName = frame.getString("file"); + boolean collapse = + frame.hasKey("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse"); int lineNumber = -1; if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) { lineNumber = frame.getInt(LINE_NUMBER_KEY); @@ -122,7 +136,7 @@ public class StackTraceHelper { if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) { columnNumber = frame.getInt(COLUMN_KEY); } - result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber); + result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse); } else if (type == ReadableType.String) { result[i] = new StackFrameImpl(null, stack.getString(i), -1, -1); } @@ -150,7 +164,9 @@ public class StackTraceHelper { if (frame.has(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) { columnNumber = frame.getInt(COLUMN_KEY); } - result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber); + boolean collapse = + frame.has("collapse") && !frame.isNull("collapse") && frame.getBoolean("collapse"); + result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber, collapse); } } catch (JSONException exception) { throw new RuntimeException(exception); diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/StackFrame.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/StackFrame.java index 78b387c9c07..89b233c13c6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/StackFrame.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/StackFrame.java @@ -36,6 +36,9 @@ public interface StackFrame { */ public String getFileName(); + /** Whether this frame is collapsed. */ + public boolean isCollapsed(); + /** Convert the stack frame to a JSON representation. */ public JSONObject toJSON(); }