feat: catch runtime error #4605

malcolm-kee · 2022-10-16T13:50:56Z

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Use finite state machine to model the overlay behavior, which can be represented with the diagram below

Currently the runtime error is caught but would need additional handling to display the source code that cause the error properly as the filename received in the ErrorEvent will be an URL prefixed with webpack:///. Not sure if we would be able to find the actual code path based on this.

Part of #3689.

Breaking Changes

Additional Info

Test results in IE10

codecov · 2022-10-16T13:56:34Z

Codecov Report

Base: 92.11% // Head: 91.98% // Decreases project coverage by -0.13% ⚠️

Coverage data is based on head (4bc3bda) compared to base (fe47a0d).
Patch coverage: 62.50% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4605      +/-   ##
==========================================
- Coverage   92.11%   91.98%   -0.14%     
==========================================
  Files          16       16              
  Lines        1661     1659       -2     
  Branches      623      618       -5     
==========================================
- Hits         1530     1526       -4     
- Misses        120      122       +2     
  Partials       11       11

Impacted Files	Coverage Δ
client-src/index.js	`81.45% <62.50%> (-0.30%)`	⬇️
lib/servers/WebsocketServer.js	`89.74% <0.00%> (-5.13%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

malcolm-kee · 2022-10-16T13:57:48Z

@alexander-akait the issue that I mentioned in the previous PR about differentiating build error vs runtime error is no longer a concern with this implementation as once any BUILD_ERROR event is emitted, the runtime error will not be displayed.

The next step I need to work on is to find out the source code that throws the runtime error (currently the info I get is the URL prefix with webpack:///), not sure if this is even possible if we do not have control on the source map.

malcolm-kee · 2022-10-16T14:03:28Z

The modeling using finite-state machine is just a random idea that come to me today. Let me know if it's involving too much change and I will revert it.

I'm using it to think through the desired behavior.

alexander-akait · 2022-10-17T09:39:48Z

client-src/overlay/state-machine.js

@@ -0,0 +1,89 @@
+import { createMachine, assign } from "@xstate/fsm";


What is size @xstate/fsm? Because extra size reduce build time

https://bundlejs.com/?q=%40xstate%2Ffsm

It's 3.94kb/1.82kb gzip.

Looks like big... Maybe we can reimplement only necessary things or use more small alternative?

What's the acceptable size? I can shop around.

I just look at @xstate/fsm's implementation, which seems to be quite minimal.

I can think of two options:

use @xstate/fsm as baseline and continue code golf it to make it as small as possible (probably by removing some error check and some convenient API).

pre-minify it before build, but I not sure if this would help the compilation efficiency.

I think the 1. will be better, we can add a new API when we will need it

Sorry for no update for more than a month! 😅

I've just created a custom createMachine while maintain enough compatibility so we can use their visualizer.

alexander-akait

Looks very good for me

@snitin315 Can you review too, also what do you think about include it in the next (or we can include it in master and get feedback faster)

snitin315

Looks great ⭐ . @malcolm-kee can you also update the webpack 4 snapshots?

@alexander-akait I think we can release it in master. next will take time.

malcolm-kee · 2022-12-02T17:13:58Z

I've updated the snapshot for webpack 4.

There are few things that I want to continue work on once this is merged:

Uncaught Promise rejection
Expose additional options to catch custom error for frameworks, e.g. the react-error-overlay has some custom code to catch React component errors
Ability to figure out the actual line of code that throws error. I suppose ability to do this would be dependent on the devtool/source map options, but I think it's still worthwhile to include that capability even it only supports certain use case only.

alexander-akait

Thank, you will do release tomorrow

alexander-akait · 2022-12-04T17:01:25Z

@malcolm-kee looks like four tests for webpack v4 is failed (not sure, why because logic is the same, maybe you are missing something, where we before have workarounds for webpack v4), can you look at them?

alexander-akait

Just to ensure, can you test it using IE11-IE10 (some developer still need it), to ensure we don't use the modern browser API and babel works correctly here, just a manual test, and thank you for your hard work

malcolm-kee · 2022-12-06T10:38:15Z

@alexander-akait is there any workflow/test case that I can refer for testing in IE? It seems like webpack code itself use arrow function, which is not supported in IE.

malcolm-kee · 2022-12-06T10:39:43Z

Also on side note, is it possible for this repo to request free account from Browserstack or Saucelab? I paid for a month subscription to do the testing above 😅

alexander-akait · 2022-12-06T11:00:46Z

@malcolm-kee You can send a request to them 😄 I use virtual machines in such case

For testing please use target: ['web', 'es5'], so webpack will generate ES5 code

malcolm-kee · 2022-12-06T11:15:34Z

Based on the fix above, overlay is displayed when there is build error.

However the runtime error is not caught, probably something wrong with window.addEventListener('error', cb) is not behaving as expected in IE. Would need to check further.

alexander-akait · 2022-12-06T12:27:45Z

Great, thank you fro this work, let's check if window.addEventListener('error', cb) is not fixable for IE, it is not a problem, because we I think no one use IE for development

alexander-akait · 2022-12-18T22:42:33Z

@malcolm-kee friendly ping ⭐

malcolm-kee · 2022-12-19T05:45:02Z

Sorry for the delayed response.
I've find out the reason runtime error catching not working and apply the fix.
The screenshots are attached in PR description.

alexander-akait

@snitin315 Can you review too, thank you

snitin315

Let's release this and gather feedback!

snitin315 · 2022-12-23T02:46:32Z

@alexander-akait you can merge and release.

alexander-akait · 2023-03-12T19:44:03Z

Sorry for big delay, let's merge and release, really very sorry

alexander-akait reviewed Oct 17, 2022

View reviewed changes

malcolm-kee force-pushed the feat/overlay-catch-runtime-error branch 2 times, most recently from 43e3f51 to 7907c8e Compare November 27, 2022 01:25

alexander-akait previously approved these changes Nov 27, 2022

View reviewed changes

snitin315 previously approved these changes Nov 28, 2022

View reviewed changes

malcolm-kee dismissed stale reviews from snitin315 and alexander-akait via 62ecee6 December 2, 2022 17:05

malcolm-kee marked this pull request as ready for review December 2, 2022 17:05

malcolm-kee requested review from hiroppy and anshumanv as code owners December 2, 2022 17:05

malcolm-kee added 5 commits December 3, 2022 04:06

feat: catch runtime error

9a60e88

fix: remove xstate

178066d

feat: display runtime error stack

d14aaf4

fix: snapshot

86c6c46

fix: update snapshot

5298888

malcolm-kee force-pushed the feat/overlay-catch-runtime-error branch from 62ecee6 to 5298888 Compare December 2, 2022 17:06

alexander-akait previously approved these changes Dec 4, 2022

View reviewed changes

fix: remove optional chaining syntax

10f69a1

malcolm-kee dismissed alexander-akait’s stale review via 10f69a1 December 5, 2022 12:45

alexander-akait reviewed Dec 5, 2022

View reviewed changes

fix: remove non IE compatible feature

d450375

malcolm-kee added 2 commits December 19, 2022 16:36

fix: handle error event in IE

cb17c06

Merge branch 'master' into feat/overlay-catch-runtime-error

4bc3bda

alexander-akait approved these changes Dec 19, 2022

View reviewed changes

snitin315 approved these changes Dec 22, 2022

View reviewed changes

alexander-akait merged commit 87a26cf into webpack:master Mar 12, 2023

redbugz mentioned this pull request Mar 23, 2023

Release 8.1.1 - disable runtimeErrors in webpack-dev-server overlay fs-webdev/create-react-app#336

Merged

malcolm-kee mentioned this pull request May 7, 2023

Document dev server overlay improvement webpack/webpack.js.org#6808

Closed

1 task

malcolm-kee deleted the feat/overlay-catch-runtime-error branch May 11, 2023 13:01

michael-s-molina mentioned this pull request Aug 2, 2023

fix: Ignores ResizeObserver errors in development mode apache/superset#24868

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: catch runtime error #4605

feat: catch runtime error #4605

malcolm-kee commented Oct 16, 2022 •

edited

codecov bot commented Oct 16, 2022 •

edited

malcolm-kee commented Oct 16, 2022

malcolm-kee commented Oct 16, 2022

alexander-akait Oct 17, 2022

malcolm-kee Oct 17, 2022

alexander-akait Oct 17, 2022

malcolm-kee Oct 17, 2022

malcolm-kee Oct 17, 2022

alexander-akait Oct 17, 2022

malcolm-kee Nov 26, 2022

alexander-akait left a comment

snitin315 left a comment

malcolm-kee commented Dec 2, 2022

alexander-akait left a comment

alexander-akait commented Dec 4, 2022 •

edited

alexander-akait left a comment

malcolm-kee commented Dec 6, 2022

malcolm-kee commented Dec 6, 2022

alexander-akait commented Dec 6, 2022

malcolm-kee commented Dec 6, 2022

alexander-akait commented Dec 6, 2022

alexander-akait commented Dec 18, 2022

malcolm-kee commented Dec 19, 2022

alexander-akait left a comment

snitin315 left a comment

snitin315 commented Dec 23, 2022

alexander-akait commented Mar 12, 2023

		@@ -0,0 +1,89 @@
		import { createMachine, assign } from "@xstate/fsm";

feat: catch runtime error #4605

feat: catch runtime error #4605

Conversation

malcolm-kee commented Oct 16, 2022 • edited

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Breaking Changes

Additional Info

codecov bot commented Oct 16, 2022 • edited

Codecov Report

malcolm-kee commented Oct 16, 2022

malcolm-kee commented Oct 16, 2022

alexander-akait Oct 17, 2022

Choose a reason for hiding this comment

malcolm-kee Oct 17, 2022

Choose a reason for hiding this comment

alexander-akait Oct 17, 2022

Choose a reason for hiding this comment

malcolm-kee Oct 17, 2022

Choose a reason for hiding this comment

malcolm-kee Oct 17, 2022

Choose a reason for hiding this comment

alexander-akait Oct 17, 2022

Choose a reason for hiding this comment

malcolm-kee Nov 26, 2022

Choose a reason for hiding this comment

alexander-akait left a comment

Choose a reason for hiding this comment

snitin315 left a comment

Choose a reason for hiding this comment

malcolm-kee commented Dec 2, 2022

alexander-akait left a comment

Choose a reason for hiding this comment

alexander-akait commented Dec 4, 2022 • edited

alexander-akait left a comment

Choose a reason for hiding this comment

malcolm-kee commented Dec 6, 2022

malcolm-kee commented Dec 6, 2022

alexander-akait commented Dec 6, 2022

malcolm-kee commented Dec 6, 2022

alexander-akait commented Dec 6, 2022

alexander-akait commented Dec 18, 2022

malcolm-kee commented Dec 19, 2022

alexander-akait left a comment

Choose a reason for hiding this comment

snitin315 left a comment

Choose a reason for hiding this comment

snitin315 commented Dec 23, 2022

alexander-akait commented Mar 12, 2023

malcolm-kee commented Oct 16, 2022 •

edited

codecov bot commented Oct 16, 2022 •

edited

alexander-akait commented Dec 4, 2022 •

edited