-
Notifications
You must be signed in to change notification settings - Fork 24.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iOS RCTFatal truncation preventing debugging #22530
Labels
Comments
It looks like you are using an older version of React Native. Please update to the latest release, v0.57 and verify if the issue still exists. The "⏪Old Version" label will be removed automatically once you edit your original post with the results of running |
2 tasks
kelset
pushed a commit
that referenced
this issue
Dec 12, 2018
Summary: Fixes #22530 As described in the issue, the previous behavior for the `RCTFatal` macro was to truncate the `reason` on the resulting `NSException` to 75 characters. This would ensure the reason would fit on a single line, but resulted in issues debugging errors that occurred in the wild, as many crash logging tools (like Sentry) discard the `name` value of the exception and use the `reason` as their primary identifier. At 75 characters, useful information like the location of the error would usually be truncated. - [x] This extends the truncation threshold to 175 characters, which should be short enough to prevent full-screen-takeover length errors, but long enough to provide useful context to the error. - [x] This adds a `userInfo` value to the resulting `NSException`. It copies over the `userInfo` from the `NSError` passed to the macro, and adds an "untruncated message" value that contains the untruncated version of the `NSException`'s reason. [iOS] [Changed] - RCTFatalExceptions now include more information in their reason and a userInfo. <!-- CATEGORY may be: - [General] - [iOS] - [Android] TYPE may be: - [Added] for new features. - [Changed] for changes in existing functionality. - [Deprecated] for soon-to-be removed features. - [Removed] for now removed features. - [Fixed] for any bug fixes. - [Security] in case of vulnerabilities. For more detail, see https://keepachangelog.com/en/1.0.0/#how MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes. EXAMPLES: [General] [Added] - Add snapToOffsets prop to ScrollView component [General] [Fixed] - Fix various issues in snapToInterval on ScrollView component [iOS] [Fixed] - Fix crash in RCTImagePicker --> Pull Request resolved: #22532 Differential Revision: D13373469 Pulled By: cpojer fbshipit-source-id: ac140d14ce76e1664869437c2c178bdd65ab6e0e
grabbou
pushed a commit
that referenced
this issue
Dec 17, 2018
Summary: Fixes #22530 As described in the issue, the previous behavior for the `RCTFatal` macro was to truncate the `reason` on the resulting `NSException` to 75 characters. This would ensure the reason would fit on a single line, but resulted in issues debugging errors that occurred in the wild, as many crash logging tools (like Sentry) discard the `name` value of the exception and use the `reason` as their primary identifier. At 75 characters, useful information like the location of the error would usually be truncated. - [x] This extends the truncation threshold to 175 characters, which should be short enough to prevent full-screen-takeover length errors, but long enough to provide useful context to the error. - [x] This adds a `userInfo` value to the resulting `NSException`. It copies over the `userInfo` from the `NSError` passed to the macro, and adds an "untruncated message" value that contains the untruncated version of the `NSException`'s reason. [iOS] [Changed] - RCTFatalExceptions now include more information in their reason and a userInfo. <!-- CATEGORY may be: - [General] - [iOS] - [Android] TYPE may be: - [Added] for new features. - [Changed] for changes in existing functionality. - [Deprecated] for soon-to-be removed features. - [Removed] for now removed features. - [Fixed] for any bug fixes. - [Security] in case of vulnerabilities. For more detail, see https://keepachangelog.com/en/1.0.0/#how MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes. EXAMPLES: [General] [Added] - Add snapToOffsets prop to ScrollView component [General] [Fixed] - Fix various issues in snapToInterval on ScrollView component [iOS] [Fixed] - Fix crash in RCTImagePicker --> Pull Request resolved: #22532 Differential Revision: D13373469 Pulled By: cpojer fbshipit-source-id: ac140d14ce76e1664869437c2c178bdd65ab6e0e
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this issue
Jun 17, 2019
Summary: Fixes facebook#22530 As described in the issue, the previous behavior for the `RCTFatal` macro was to truncate the `reason` on the resulting `NSException` to 75 characters. This would ensure the reason would fit on a single line, but resulted in issues debugging errors that occurred in the wild, as many crash logging tools (like Sentry) discard the `name` value of the exception and use the `reason` as their primary identifier. At 75 characters, useful information like the location of the error would usually be truncated. - [x] This extends the truncation threshold to 175 characters, which should be short enough to prevent full-screen-takeover length errors, but long enough to provide useful context to the error. - [x] This adds a `userInfo` value to the resulting `NSException`. It copies over the `userInfo` from the `NSError` passed to the macro, and adds an "untruncated message" value that contains the untruncated version of the `NSException`'s reason. [iOS] [Changed] - RCTFatalExceptions now include more information in their reason and a userInfo. <!-- CATEGORY may be: - [General] - [iOS] - [Android] TYPE may be: - [Added] for new features. - [Changed] for changes in existing functionality. - [Deprecated] for soon-to-be removed features. - [Removed] for now removed features. - [Fixed] for any bug fixes. - [Security] in case of vulnerabilities. For more detail, see https://keepachangelog.com/en/1.0.0/#how MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes. EXAMPLES: [General] [Added] - Add snapToOffsets prop to ScrollView component [General] [Fixed] - Fix various issues in snapToInterval on ScrollView component [iOS] [Fixed] - Fix crash in RCTImagePicker --> Pull Request resolved: facebook#22532 Differential Revision: D13373469 Pulled By: cpojer fbshipit-source-id: ac140d14ce76e1664869437c2c178bdd65ab6e0e
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
(Not using the latest release yet, but have confirmed this issue exists in latest as well)
Environment
React Native Environment Info:
Summary
TLDR; The very low "max message length" of 75 at this line in
RCTAssert.m
rendersRCTFatal
errors very difficult to debug.Description
What we're seeing is that errors captured by the iOS built-in crash analytics and by Sentry.io are using the
NSException.message
to describe the message and discarding thename
value. As such, we're only seeing the 75 character truncated value. Given how much of that string is dedicated to boilerplate information about the exception, we wind up with exception logs likeRCTFatalException: Exception '*** -[__NSArrayM setObject:atIndexedSubscript:]: object cannot be nil' was thrown while invokin...
that truncate just before the actually useful information for debugging would be printed.There isn't an especially compelling reason that I know of for keeping this value truncated as short as 75 characters. (Indeed, IMO there isn't really a reason to truncate it at all, but I expect there are cases that demand that that I'm not aware of.) I'm going to make a branch and pull-request that will both extend this truncation length and add information to the
userinfo
dictionary on the exception.Reproducible Demo
Cause any exception and view the value in the RCTFatalException that is thrown.
The text was updated successfully, but these errors were encountered: