Skip to content
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

Unhandled promise rejection - attach non-standard Error object stack info if possible #42079

Closed
wants to merge 5 commits into from

Conversation

ospfranco
Copy link
Contributor

Summary:

This is a continuation of my last PR which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

Changelog:

Test Plan:

Test any unhandled promise rejection with a non-standard error (line 23, toString must not return [object Error]) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Dec 27, 2023
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I don't know this part of the framework as well as other people, so I'll let them have another round of review.

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ospfranco
Copy link
Contributor Author

@javache reviewed the last PR

Copy link

Choose a reason for hiding this comment

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

I might be mistaken but shouldn't the rejection.stack unwinding be done optimally before the final JSON.stringify((rejection: $FlowFixMe))?

Copy link
Contributor Author

@ospfranco ospfranco Dec 27, 2023

Choose a reason for hiding this comment

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

No, I don't think so, stringify does not unwind the stack or show it in the logbox . The stringify call is only for extracting a message.

Copy link

This pull request was successfully merged by @ospfranco in 655b12d.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Dec 28, 2023
huntie pushed a commit that referenced this pull request Jan 2, 2024
…info if possible (#42079)

Summary:
This is a continuation of my [last PR](#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: #42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
…info if possible (facebook#42079)

Summary:
This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: facebook#42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
yayvery pushed a commit to discord/react-native that referenced this pull request Jan 10, 2024
…info if possible (facebook#42079)

Summary:
This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: facebook#42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
yayvery pushed a commit to discord/react-native that referenced this pull request Jan 11, 2024
…info if possible (facebook#42079)

Summary:
This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: facebook#42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
huntie pushed a commit that referenced this pull request Jan 15, 2024
…info if possible (#42079)

Summary:
This is a continuation of my [last PR](#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: #42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
hurali97 added a commit that referenced this pull request Jan 15, 2024
Apply stack Symbolication patch part 1 & 2 to 0.71-stable (#41377) (#42079) (original #40914)
Flewp pushed a commit to discord/react-native that referenced this pull request Mar 9, 2024
…object stack info if possible (facebook#42079)

Summary:
This is a continuation of my [last PR](facebook#40914) which improved the symbolication of unhandled promise rejections.

While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: facebook#42079

Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.

Here is the one I got before and after this change:

<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />

<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />

Reviewed By: huntie

Differential Revision: D52431711

Pulled By: cipolleschi

fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants