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

Consistent error handling for addon-interactions #16585

Closed
shilman opened this issue Nov 4, 2021 · 6 comments
Closed

Consistent error handling for addon-interactions #16585

shilman opened this issue Nov 4, 2021 · 6 comments

Comments

@shilman
Copy link
Member

shilman commented Nov 4, 2021

Currently we handle errors differently between errors thrown on instrumented functions and errors thrown on non-instrumented functions:

const Instrumented = Template.bind({});
Instrumented.play = async ({ canvasElement }) => {
  const taskInput = await findByRole(canvas, 'textbox');
  await fireEvent.change(taskInput, {
    target: { value: 'Fix bug in the textarea error state' },
  });
}

const NonInstrumented = Template.bind({});
NonInstrumented.play = async ({ canvasElement }) => {
  throw new Error('hahaha');
};

The former shows an error in the interactions tab:

InboxScreen_-_Default_⋅_Storybook

The latter shows a typical storybook error:

InboxScreen_-_Default_⋅_Storybook

Should this handling be consistent?

@ghengeveld
Copy link
Member

ghengeveld commented Nov 4, 2021

Preferably we wouldn't show a red box for simple errors at all, but rather something like the interactions panel. However, the interactions addon might not be installed, even when using the play function and instrumented libraries. That means we can't simply catch everything thrown in the play function, because doing so would gobble it up and go nowhere.

We could simply catch it, log the error to console and call it a day.
We could detect whether the Interactions addon is installed and only catch if it is.
We could introduce a bespoke UI component in the Storybook manager to surface exceptions. Basically a nicer version of the redbox, somewhere in a modal or something. This could tie in nicely with a render phase indicator.

@tmeasday
Copy link
Member

tmeasday commented Nov 4, 2021

I think @ghengeveld's ideas make sense; I would just say that for now whatever we do I think we should be consistent between normal and intercepted errors. For 6.4 that probably means logging to the console and showing in the interactions panel.

@shilman
Copy link
Member Author

shilman commented Nov 5, 2021

Huzzah!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-beta.29 containing PR #16590 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

@shilman shilman closed this as completed Nov 5, 2021
@ghengeveld
Copy link
Member

Reopening because the PR that handles this is still pending: #16592

@ghengeveld ghengeveld reopened this Nov 11, 2021
@tmeasday
Copy link
Member

Sorry not sure why I referenced this issue

@shilman shilman closed this as completed Nov 12, 2021
@shilman
Copy link
Member Author

shilman commented Jul 6, 2022

Ta-da!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.11 containing PR #16592 that references this issue. Upgrade today to the @future NPM tag to try it out!

npx sb upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants