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

Error handling broken in Storybook 5.2 #8087

Closed
shilman opened this issue Sep 16, 2019 · 9 comments
Closed

Error handling broken in Storybook 5.2 #8087

shilman opened this issue Sep 16, 2019 · 9 comments
Assignees
Labels
bug core patch:yes Bugfix & documentation PR that need to be picked to main branch
Milestone

Comments

@shilman
Copy link
Member

shilman commented Sep 16, 2019

In 5.2, errors are silently swallowed.

In 5.1, story errors would result in the stack trace showing up in the main window:

Storybook

To repro in monorepo:

cd examples/official-storybook
yarn storybook

Open Core > Errors > story throws exception (and compare to release/5.1 branch).

@shilman
Copy link
Member Author

shilman commented Sep 16, 2019

I believe the problem is related to preview hooks. First, it seemed like it was source-loader related, but when I removed the source-loader (by removing the addon-docs preset and also *.stories.mdx in official-storybook), I see the following error in the stack trace:

    hooks.nextHookIndex = 0;
    var prevContext = _global.window.STORYBOOK_HOOKS_CONTEXT;
    _global.window.STORYBOOK_HOOKS_CONTEXT = hooks;
    var result = fn.apply(void 0, arguments);
    _global.window.STORYBOOK_HOOKS_CONTEXT = prevContext;

I believe this code needs an exception handler and ultimately needs to call showError

@shilman shilman added this to the 5.2.0 milestone Sep 16, 2019
@Hypnosphi Hypnosphi self-assigned this Sep 16, 2019
@Hypnosphi
Copy link
Member

@tmeasday do you think we can find a way to enable chromatic testing for those error stories?

@tmeasday
Copy link
Member

@Hypnosphi do you have any ideas about how that could work? Chromatic is designed so that an error needs to be resolved before you can start accepting/denying changes as a matter of course.

In any case we want to capture the actual error div etc which falls outside of the way that Chromatic captures (we get the contents of the #root element). One way we could do it is using an iframe maybe, although that's a bit of a hack.

@Hypnosphi
Copy link
Member

yes, I think I can just add a story that renders iframes with other stories

@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Sep 17, 2019
@shilman
Copy link
Member Author

shilman commented Sep 17, 2019

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.0 containing PR #8097 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman
Copy link
Member Author

shilman commented Sep 17, 2019

Yippee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.1 containing PR #8097 that references this issue. Upgrade today to try it out!

@shilman shilman reopened this Sep 17, 2019
@shilman
Copy link
Member Author

shilman commented Sep 17, 2019

Probably needs another fix, but better to get something out for the influx of new users. Will close this after we finalize what's in #8100

@shilman shilman modified the milestones: 5.2.0, 5.2.x Sep 23, 2019
@shilman
Copy link
Member Author

shilman commented Oct 4, 2019

Shiver me timbers!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.9 containing PR #8100 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 4, 2019
@shilman
Copy link
Member Author

shilman commented Oct 7, 2019

Yowza!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.2 containing PR #8100 that references this issue. Upgrade today to try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug core patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

No branches or pull requests

3 participants