Skip to content

Commit

Permalink
cleanup RedBox message and stack output (#24662)
Browse files Browse the repository at this point in the history
Summary:
Cleanup RedBox messages and stack traces. This PR consists of 2 changes (I'm good with splitting them up if you'd like):

- [general] filter out some of the internal callsites from the symbolicated stack (I thought about using monospace font for title with code frame, but it looks weird)
- [ios][android] strip ANSI characters (coming from colored Babel code frame) from the error message

I think it's ok to strip it inside native handlers so we can still have a colorful code frame in the terminal output.

**JS Code frame:**

|before|after|
|--|--|
|<img width="400" alt="Screenshot 2019-04-30 at 12 32 05" src="https://user-images.githubusercontent.com/5106466/56956590-ef678d80-6b44-11e9-9019-6801f050ab0d.png">|<img width="400" alt="Screenshot 2019-04-30 at 12 52 43" src="https://user-images.githubusercontent.com/5106466/56957302-f42d4100-6b46-11e9-869b-ea9c7ce5b90f.png">|

|before|after|
|--|--|
|![image](https://user-images.githubusercontent.com/5106466/56959472-c8618980-6b4d-11e9-84be-6261d8375f4a.png)|![image](https://user-images.githubusercontent.com/5106466/56959463-bc75c780-6b4d-11e9-9d8b-25ffe46c87cf.png)|

**Filtered stack traces:**

|before|after|
|--|--|
|<img width="50%" alt="Screenshot 2019-04-30 at 12 27 21" src="https://user-images.githubusercontent.com/5106466/56956641-0908d500-6b45-11e9-8cdc-8c2a34a071e5.png"><img width="50%" alt="Screenshot 2019-04-30 at 12 27 28" src="https://user-images.githubusercontent.com/5106466/56956642-0908d500-6b45-11e9-921c-fabfb8515cc0.png">|<img width="100%" alt="Screenshot 2019-04-30 at 12 26 55" src="https://user-images.githubusercontent.com/5106466/56956650-0efeb600-6b45-11e9-9f5f-f10dd69580d1.png">|

There's still a lot of places that are hard to read, but I think this is a good start towards more readable errors.

cc cpojer

[General][Changed] - Cleanup RedBox message and stack output
Pull Request resolved: #24662

Differential Revision: D15147571

Pulled By: cpojer

fbshipit-source-id: 1de4e521af988fa7fc709b6accd0ddd984388e72
  • Loading branch information
thymikee authored and facebook-github-bot committed Apr 30, 2019
1 parent b92f30d commit 49d26eb
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
12 changes: 11 additions & 1 deletion Libraries/Core/ExceptionsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@

import type {ExtendedError} from 'parseErrorStack';

const INTERNAL_CALLSITES_REGEX = new RegExp(
[
'/Libraries/Renderer/oss/ReactNativeRenderer-dev\\.js$',
'/Libraries/BatchedBridge/MessageQueue\\.js$',
].join('|'),
);

/**
* Handles the developer-visible aspect of errors and exceptions
*/
Expand All @@ -38,9 +45,12 @@ function reportException(e: ExtendedError, isFatal: boolean) {
symbolicateStackTrace(stack)
.then(prettyStack => {
if (prettyStack) {
const stackWithoutInternalCallsites = prettyStack.filter(
frame => frame.file.match(INTERNAL_CALLSITES_REGEX) === null,
);
ExceptionsManager.updateExceptionMessage(
e.message,
prettyStack,
stackWithoutInternalCallsites,
currentExceptionID,
);
} else {
Expand Down
20 changes: 15 additions & 5 deletions React/Modules/RCTRedBox.m
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ - (instancetype)initWithFrame:(CGRect)frame

CGFloat buttonWidth = self.bounds.size.width / 4;
CGFloat bottomButtonHeight = self.bounds.size.height - buttonHeight - [self bottomSafeViewHeight];

dismissButton.frame = CGRectMake(0, bottomButtonHeight, buttonWidth, buttonHeight);
reloadButton.frame = CGRectMake(buttonWidth, bottomButtonHeight, buttonWidth, buttonHeight);
copyButton.frame = CGRectMake(buttonWidth * 2, bottomButtonHeight, buttonWidth, buttonHeight);
Expand All @@ -148,11 +148,11 @@ - (instancetype)initWithFrame:(CGRect)frame
[rootView addSubview:copyButton];
[rootView addSubview:extraButton];
[rootView addSubview:topBorder];

UIView *bottomSafeView = [UIView new];
bottomSafeView.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
bottomSafeView.frame = CGRectMake(0, self.bounds.size.height - [self bottomSafeViewHeight], self.bounds.size.width, [self bottomSafeViewHeight]);

[rootView addSubview:bottomSafeView];
}
return self;
Expand All @@ -176,14 +176,24 @@ - (void)dealloc
[[NSNotificationCenter defaultCenter] removeObserver:self];
}

- (NSString *)stripAnsi:(NSString *)text
{
NSError *error = nil;
NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern:@"\\x1b\\[[0-9;]*m" options:NSRegularExpressionCaseInsensitive error:&error];
return [regex stringByReplacingMatchesInString:text options:0 range:NSMakeRange(0, [text length]) withTemplate:@""];
}

- (void)showErrorMessage:(NSString *)message withStack:(NSArray<RCTJSStackFrame *> *)stack isUpdate:(BOOL)isUpdate
{
// Remove ANSI color codes from the message
NSString *messageWithoutAnsi = [self stripAnsi:message];

// Show if this is a new message, or if we're updating the previous message
if ((self.hidden && !isUpdate) || (!self.hidden && isUpdate && [_lastErrorMessage isEqualToString:message])) {
if ((self.hidden && !isUpdate) || (!self.hidden && isUpdate && [_lastErrorMessage isEqualToString:messageWithoutAnsi])) {
_lastStackTrace = stack;
// message is displayed using UILabel, which is unable to render text of
// unlimited length, so we truncate it
_lastErrorMessage = [message substringToIndex:MIN((NSUInteger)10000, message.length)];
_lastErrorMessage = [messageWithoutAnsi substringToIndex:MIN((NSUInteger)10000, messageWithoutAnsi.length)];

[_stackTraceTableView reloadData];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ public View getView(int position, View convertView, ViewGroup parent) {
? (TextView) convertView
: (TextView) LayoutInflater.from(parent.getContext())
.inflate(R.layout.redbox_item_title, parent, false);
title.setText(mTitle);
// Remove ANSI color codes from the title
title.setText(mTitle.replaceAll("\\x1b\\[[0-9;]*m", ""));
return title;
} else {
if (convertView == null) {
Expand Down

3 comments on commit 49d26eb

@aleclarson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thymikee Thanks for this! ❤️

@turnrye
Copy link
Contributor

@turnrye turnrye commented on 49d26eb Jun 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thymikee this is great!

@mayconmesquita
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow good job!

Please sign in to comment.