Manage redbox updates by comparing exception IDs

Summary: Refines the condition for ignoring updates to an already-open redbox on iOS. Previously we would only update the stack trace if the message string in the update was exactly the same as in the initial redbox. With this diff we rely on the `exceptionId` parameter instead, and only fall back to string comparison if it's omitted (i.e. for non-JS uses of redbox on iOS). NOTE: `exceptionId` is part of the existing ExceptionsManager API and is already supported on Android. Reviewed By: sammy-SC Differential Revision: D17226179 fbshipit-source-id: 5940110bf4e4358a8a1b3477e8d2cf8b224dd9f8
facebook · Sep 6, 2019 · 850a835 · 850a835
1 parent 2510632
commit 850a835
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 19 deletions.
diff --git a/React/CoreModules/RCTExceptionsManager.mm b/React/CoreModules/RCTExceptionsManager.mm
@@ -38,7 +38,7 @@ - (instancetype)initWithDelegate:(id<RCTExceptionsManagerDelegate>)delegate
                   stack:(NSArray<NSDictionary *> *)stack
                   exceptionId:(double)exceptionId)
 {
-  [_bridge.redBox showErrorMessage:message withStack:stack];
+  [_bridge.redBox showErrorMessage:message withStack:stack errorCookie:((int)exceptionId)];
 
   if (_delegate) {
     [_delegate handleSoftJSExceptionWithMessage:message stack:stack exceptionId:[NSNumber numberWithDouble:exceptionId]];
@@ -49,7 +49,7 @@ - (instancetype)initWithDelegate:(id<RCTExceptionsManagerDelegate>)delegate
                   stack:(NSArray<NSDictionary *> *)stack
                   exceptionId:(double) exceptionId)
 {
-  [_bridge.redBox showErrorMessage:message withStack:stack];
+  [_bridge.redBox showErrorMessage:message withStack:stack errorCookie:((int)exceptionId)];
 
   if (_delegate) {
     [_delegate handleFatalJSExceptionWithMessage:message stack:stack exceptionId:[NSNumber numberWithDouble:exceptionId]];
@@ -70,7 +70,7 @@ - (instancetype)initWithDelegate:(id<RCTExceptionsManagerDelegate>)delegate
                   stack:(NSArray<NSDictionary *> *)stack
                   exceptionId:(double)exceptionId)
 {
-  [_bridge.redBox updateErrorMessage:message withStack:stack];
+  [_bridge.redBox updateErrorMessage:message withStack:stack errorCookie:((int)exceptionId)];
 
   if (_delegate && [_delegate respondsToSelector:@selector(updateJSExceptionWithMessage:stack:exceptionId:)]) {
     [_delegate updateJSExceptionWithMessage:message stack:stack exceptionId:[NSNumber numberWithDouble:exceptionId]];

diff --git a/React/Modules/RCTRedBox.h b/React/Modules/RCTRedBox.h
@@ -20,10 +20,15 @@
 - (void)showErrorMessage:(NSString *)message;
 - (void)showErrorMessage:(NSString *)message withDetails:(NSString *)details;
 - (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack;
+- (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack errorCookie:(int)errorCookie;
 - (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack;
 - (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack;
+- (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie;
+- (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie;
 - (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack;
 - (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack;
+- (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie;
+- (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie;
 
 - (void)dismiss;
 

diff --git a/React/Modules/RCTRedBox.m b/React/Modules/RCTRedBox.m
@@ -37,10 +37,12 @@ @implementation RCTRedBoxWindow
     UITableView *_stackTraceTableView;
     NSString *_lastErrorMessage;
     NSArray<RCTJSStackFrame *> *_lastStackTrace;
+    int _lastErrorCookie;
 }
 
 - (instancetype)initWithFrame:(CGRect)frame
 {
+    _lastErrorCookie = -1;
     if ((self = [super initWithFrame:frame])) {
 #if TARGET_OS_TV
         self.windowLevel = UIWindowLevelAlert + 1000;
@@ -183,17 +185,25 @@ - (NSString *)stripAnsi:(NSString *)text
     return [regex stringByReplacingMatchesInString:text options:0 range:NSMakeRange(0, [text length]) withTemplate:@""];
 }
 
-- (void)showErrorMessage:(NSString *)message withStack:(NSArray<RCTJSStackFrame *> *)stack isUpdate:(BOOL)isUpdate
+- (void)showErrorMessage:(NSString *)message withStack:(NSArray<RCTJSStackFrame *> *)stack isUpdate:(BOOL)isUpdate errorCookie:(int)errorCookie
 {
     // 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:messageWithoutAnsi])) {
+    BOOL isNew = self.hidden && !isUpdate;
+    BOOL isUpdateForSameMessage = !isNew && (
+      !self.hidden && isUpdate && (
+        (errorCookie == -1 && [_lastErrorMessage isEqualToString:messageWithoutAnsi]) ||
+        (errorCookie == _lastErrorCookie)
+      )
+    );
+    if (isNew || isUpdateForSameMessage) {
         _lastStackTrace = stack;
         // message is displayed using UILabel, which is unable to render text of
         // unlimited length, so we truncate it
         _lastErrorMessage = [messageWithoutAnsi substringToIndex:MIN((NSUInteger)10000, messageWithoutAnsi.length)];
+        _lastErrorCookie = errorCookie;
 
         [_stackTraceTableView reloadData];
 
@@ -440,54 +450,80 @@ - (void)showError:(NSError *)error
 {
     [self showErrorMessage:error.localizedDescription
                withDetails:error.localizedFailureReason
-                     stack:error.userInfo[RCTJSStackTraceKey]];
+                     stack:error.userInfo[RCTJSStackTraceKey]
+                     errorCookie:-1];
 }
 
 - (void)showErrorMessage:(NSString *)message
 {
-    [self showErrorMessage:message withParsedStack:nil isUpdate:NO];
+    [self showErrorMessage:message withParsedStack:nil isUpdate:NO errorCookie:-1];
 }
 
 - (void)showErrorMessage:(NSString *)message withDetails:(NSString *)details
 {
-    [self showErrorMessage:message withDetails:details stack:nil];
+    [self showErrorMessage:message withDetails:details stack:nil errorCookie:-1];
 }
 
-- (void)showErrorMessage:(NSString *)message withDetails:(NSString *)details stack:(NSArray<RCTJSStackFrame *> *)stack {
+- (void)showErrorMessage:(NSString *)message withDetails:(NSString *)details stack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie {
     NSString *combinedMessage = message;
     if (details) {
         combinedMessage = [NSString stringWithFormat:@"%@\n\n%@", message, details];
     }
-    [self showErrorMessage:combinedMessage withParsedStack:stack isUpdate:NO];
+    [self showErrorMessage:combinedMessage withParsedStack:stack isUpdate:NO errorCookie:errorCookie];
 }
 
 - (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack
+{
+    [self showErrorMessage:message withRawStack:rawStack errorCookie:-1];
+}
+
+- (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack errorCookie:(int)errorCookie
 {
     NSArray<RCTJSStackFrame *> *stack = [RCTJSStackFrame stackFramesWithLines:rawStack];
-    [self showErrorMessage:message withParsedStack:stack isUpdate:NO];
+    [self showErrorMessage:message withParsedStack:stack isUpdate:NO errorCookie:errorCookie];
 }
 
 - (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack
 {
-    [self showErrorMessage:message withParsedStack:[RCTJSStackFrame stackFramesWithDictionaries:stack] isUpdate:NO];
+    [self showErrorMessage:message withStack:stack errorCookie:-1];
 }
 
 - (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack
 {
-    [self showErrorMessage:message withParsedStack:[RCTJSStackFrame stackFramesWithDictionaries:stack] isUpdate:YES];
+    [self updateErrorMessage:message withStack:stack errorCookie:-1];
+}
+
+- (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie
+{
+    [self showErrorMessage:message withParsedStack:[RCTJSStackFrame stackFramesWithDictionaries:stack] isUpdate:NO errorCookie:errorCookie];
+}
+
+- (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie
+{
+    [self showErrorMessage:message withParsedStack:[RCTJSStackFrame stackFramesWithDictionaries:stack] isUpdate:YES errorCookie:errorCookie];
 }
 
 - (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack
 {
-    [self showErrorMessage:message withParsedStack:stack isUpdate:NO];
+    [self showErrorMessage:message withParsedStack:stack errorCookie:-1];
 }
 
 - (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack
 {
-    [self showErrorMessage:message withParsedStack:stack isUpdate:YES];
+    [self updateErrorMessage:message withParsedStack:stack errorCookie:-1];
+}
+
+- (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie
+{
+    [self showErrorMessage:message withParsedStack:stack isUpdate:NO errorCookie:errorCookie];
 }
 
-- (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack isUpdate:(BOOL)isUpdate
+- (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie
+{
+    [self showErrorMessage:message withParsedStack:stack isUpdate:YES errorCookie:errorCookie];
+}
+
+- (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack isUpdate:(BOOL)isUpdate errorCookie:(int)errorCookie
 {
     dispatch_async(dispatch_get_main_queue(), ^{
         if (self->_extraDataViewController == nil) {
@@ -510,7 +546,8 @@ - (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStack
         errorInfo = [self _customizeError:errorInfo];
         [self->_window showErrorMessage:errorInfo.errorMessage
                               withStack:errorInfo.stack
-                               isUpdate:isUpdate];
+                               isUpdate:isUpdate
+                               errorCookie:errorCookie];
     });
 }
 
@@ -592,15 +629,20 @@ @implementation RCTRedBox
 
 + (NSString *)moduleName { return nil; }
 - (void)registerErrorCustomizer:(id<RCTErrorCustomizer>)errorCustomizer {}
-- (void)showError:(NSError *)message {}
+- (void)showError:(NSError *)error {}
 - (void)showErrorMessage:(NSString *)message {}
 - (void)showErrorMessage:(NSString *)message withDetails:(NSString *)details {}
 - (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack {}
+- (void)showErrorMessage:(NSString *)message withRawStack:(NSString *)rawStack errorCookie:(int)errorCookie {}
 - (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack {}
 - (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack {}
+- (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie {}
+- (void)updateErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack errorCookie:(int)errorCookie {}
 - (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack {}
 - (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack {}
-- (void)showErrorMessage:(NSString *)message withStack:(NSArray<NSDictionary *> *)stack isUpdate:(BOOL)isUpdate {}
+- (void)showErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie {}
+- (void)updateErrorMessage:(NSString *)message withParsedStack:(NSArray<RCTJSStackFrame *> *)stack errorCookie:(int)errorCookie {}
+
 - (void)dismiss {}
 
 @end