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

Core: Pass proper stack of an error #15864

Merged
merged 3 commits into from
Sep 6, 2021
Merged

Conversation

extempl
Copy link
Contributor

@extempl extempl commented Aug 17, 2021

Reopening #14336

Issue: Stack of an error is not the error's, but the storybook's.

What I did

Pass stack explicitly (toString does not include it).

How to test

  • Is this testable with Jest or Chromatic screenshots? - Passively?
  • Does this need a new example in the kitchen sink apps? - No
  • Does this need an update to the documentation? - No

If your answer is yes to any of these, please make sure to include it in your PR.

@MichaelArestad
Copy link
Contributor

This looks identical to the previous PR you made, #14336. Can you provide an example or detailed steps to reproduce the issue you are having?

@extempl
Copy link
Contributor Author

extempl commented Aug 25, 2021

@MichaelArestad @yannbf indeed it is identical, this is the only way I can 'reopen' it, since it was closed.
The example of the issue and solution is in the latest comment: #14336 (comment)

The issue is that (new Error).toString does not contain a stacktrace, but message only. The stack trace is visible after that does not help to solve any problems occured because of the error, as it shows nothing, but the fact that something went wrong and the error message.
You can see that in example when you attaching e.stack - it shows the method where the original error occured, not where it was re-thrown.
So I need to have an actual stack trace, and not the storybook-logger which is only shows where the exception was re-thrown.
Also my fix is backwards compatible, I do not really see the reason to not merge it.

@yannbf yannbf self-requested a review August 31, 2021 22:19
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @extempl! It certainly makes things much better. I simulated this error by calling a function that does not exist in a .stories file.

Here's the result prior to your fix:
image

Here's with your fix:
image

Great contribution! 👏

@shilman
Copy link
Member

shilman commented Aug 31, 2021

@yannbf any chance you can help get CI to pass?

@nx-cloud
Copy link

nx-cloud bot commented Sep 1, 2021

Nx Cloud Report

CI ran the following commands for commit 9993215. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@yannbf
Copy link
Member

yannbf commented Sep 5, 2021

@shilman this seems good to go!

@shilman shilman added this to the 6.4 PRs milestone Sep 5, 2021
@shilman shilman changed the title Pass proper stack of an error Core: Pass proper stack of an error Sep 6, 2021
@shilman shilman merged commit a6ba730 into storybookjs:next Sep 6, 2021
@extempl extempl deleted the patch-1 branch October 15, 2021 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants