Emit messages when stories fail to render

[tmeasday]

Issue:

There is no way for external tools or the manager context to know when a story fails to render

What I did

Emitted two events when the story errors or throws.

How to test

I added stories for the two cases, however there are a couple of problems/questions:

1. The current use of emotion's ThemeProvider in official storybook means that my "error" story ends up throwing rather than triggering the react layer's error code. Any ideas on how to work around this @ndelangen? -- I don't want this decorator to apply for this story -- Perhaps we need to do something hacky like addDecorator(story, ({kind})) => kind === 'Errors' ? story() : <ThemeProvider>...)

2. I'd like to also throw the error if the story throws sometime after the initial render. Is this out of scope? It would be useful for Chromatic to know that a component has errored even if it does it async. This would require work from all the view layers so perhaps is a separate PR. Could be out of scope for storybook in any case.

3. Is there a way to test the event is emitted correctly?

[storybook-safe-bot]

	Fails
🚫	PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS

[ndelangen]

Please explain why the <ThemeProvider> is involved here at all?

Would a try-catch decorator work here?

[tmeasday]

Please explain why the is involved here at all?

Sorry, that was a bit low on detail.

So the React integration has some code that calls setError if a story returns something that is not renderable by React. However, the use of the <ThemeProvider> decorator means that when a story wrapped by it returns something not renderable, the ThemeProvider internally breaks and an exception is thrown. Ultimately this leads to setException being called (as you can see from this PR, there is a subtle difference in the meaning).

I think we need a story that results in setError being called. AFAICT, the only way we can make this happen is to not apply the decorator to this story. Can you think of a better way than my ternary above?

[tmeasday]

What do you think @ndelangen? Also any thoughts on how to test it?

[codecov]

Codecov Report

Merging #3967 into master will increase coverage by 2.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3967      +/-   ##
==========================================
+ Coverage   39.66%   41.68%   +2.02%     
==========================================
  Files         433      433              
  Lines        5445     5421      -24     
  Branches      740      739       -1     
==========================================
+ Hits         2160     2260     +100     
+ Misses       2903     2796     -107     
+ Partials      382      365      -17

Impacted Files	Coverage Δ
lib/core-events/index.js	100% <ø> (ø)	⬆️
lib/core/src/client/preview/start.js	90.27% <100%> (+90.27%)	⬆️
lib/core/src/client/preview/story_store.js	94.11% <0%> (+3.92%)	⬆️
lib/core/src/client/preview/config_api.js	55.55% <0%> (+55.55%)	⬆️
lib/core/src/client/preview/actions.js	60% <0%> (+60%)	⬆️
lib/core/src/client/preview/reducer.js	81.81% <0%> (+81.81%)	⬆️
lib/core/src/client/preview/syncUrlWithStore.js	85.71% <0%> (+85.71%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6f57bd...a42250f. Read the comment docs.

[tmeasday]

I added a test and sorted out the theming thing (let me know if you are happy with that @ndelangen)

[tmeasday]

@igor-dv these stories I've added are guaranteed to fail storyshots, as they throw errors after all ;)

I want to change them to not be included in our storyshots test suite. I can do this via looking into navigator.userAgent but it'd be nice to be able to use params for it, like:

.add('error', () => null, { storyshots: { disable: true } })

Maybe this is a separate PR, but I am wondering if we can work together to add that API to storyshots? I think we need to tweak the way it gets stuff out of the store for it to have direct access to a stories parameters

[igor-dv]

Yeah, it's a nice API. What if you would like to exclude stories from one test but include them into another one?

[tmeasday]

What if you would like to exclude stories from one test but include them into another one?

Do you mean in a different "suite"? We could also add something like { storyshots: { suite: 'X' } } -- would that be good?

[tmeasday]

This is ready to merge I think, we can add the params API to storyshots later #4010

[thedamon]

I'm confused... is this now a feature of storyshots?
If so.. a way to distinguish between storyshots and imageSnapshot from storyshots-puppeteer in these story options would be a, maze, ing.