Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: prevent overlay for errors caught by React error boundaries #5431

Merged

Conversation

negimox
Copy link
Contributor

@negimox negimox commented Mar 11, 2025

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

fixes #5397

Breaking Changes

Additional Info

webpack_dev_fix-2025-03-11_20.55.45.mp4

Copy link

linux-foundation-easycla bot commented Mar 11, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@negimox
Copy link
Contributor Author

negimox commented Mar 11, 2025

Hello, @alexander-akait @snitin315 can you verify if this solution for issue #5411 is appropriate?

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

We watch only runtime error here, but title ask about don't show it for all error which was caught by react-error-boundary... Maybe it is enough, can you test using this repo - https://github.com/OliverJAsh/react-error-boundary-uncaught-issue/tree/webpack-dev-server like was written in the issue?

@negimox
Copy link
Contributor Author

negimox commented Mar 11, 2025

Hello, @alexander-akait

Can you test using this repo - https://github.com/OliverJAsh/react-error-boundary-uncaught-issue/tree/webpack-dev-server like was written in the issue?

Yes, I have tested with the repo attached in the issue and it was working i.e. overlay doesn't show. For reference of the test you can check the .mp4 video under additional info where that repo is used.

Copy link

codecov bot commented Mar 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.42%. Comparing base (af6bd68) to head (7b8964c).
Report is 108 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5431      +/-   ##
==========================================
- Coverage   90.29%   83.42%   -6.88%     
==========================================
  Files          15       13       -2     
  Lines        1577     1979     +402     
  Branches      601      723     +122     
==========================================
+ Hits         1424     1651     +227     
- Misses        140      295     +155     
- Partials       13       33      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alexander-akait
Copy link
Member

Let's fix lint error

@negimox
Copy link
Contributor Author

negimox commented Mar 12, 2025

Hello @alexander-akait, lint errors are resolved now

@negimox negimox requested a review from alexander-akait March 12, 2025 03:28
@negimox
Copy link
Contributor Author

negimox commented Mar 12, 2025

Hi @alexander-akait, apologies for the repeated mentions. I noticed that one of the checks is failing—specifically, Test - windows-latest - Node v20.x, Webpack latest (4/4). Based on the error log, it seems the failure is occurring in the test/e2e/api.test.js file for cases when the port is either null or undefined, resulting in the error: "listen EACCES: permission denied 0.0.0.0:60000." Could you please help clarify if these errors might be related to my changes in the client/overlay file?

@alexander-akait
Copy link
Member

alexander-akait commented Mar 13, 2025

Ignore it, this tests are unstable in some situations, if you want to rewrite tests - PR welcome, but it is not related to your changes in this PR, so I will merge

@alexander-akait alexander-akait merged commit 8c1abc9 into webpack:master Mar 13, 2025
51 of 53 checks passed
@cakidnyc
Copy link

Did this update cause a regression for plain vanilla JS users? Everything was working fine in "webpack-dev-server": "~5.2.0", and now with 5.2.1, the overlay never shows, see this console error, which looks like the new code added:

overlay.js:602 Uncaught TypeError: Cannot read properties of null (reading 'stack')
at eval (overlay.js:602:1)

Once again, we don't use react. Just plain, plain as plain vanilla JS.

@alexander-akait
Copy link
Member

@cakidnyc Can you create reproducible test repo?

@alexander-akait
Copy link
Member

alexander-akait commented Mar 28, 2025

Reproduced, looks like your code throws null, i.e. throw null; I will fix it, but I recommended check out your code, because it is very strange to throw null

@cakidnyc
Copy link

Reproduced, looks like your code throws null, i.e. throw null; I will fix it, but I recommended check out your code, because it is very strange to throw null

Thanks. I agree, but that error is coming from a 3rd party lib. To quote (almost) Forest Gump's Mom: "UI Dev is like a box of chocolates ... you never know what you are going to get".

@dh049
Copy link

dh049 commented Apr 9, 2025

Did this update cause a regression for plain vanilla JS users? Everything was working fine in "webpack-dev-server": "~5.2.0", and now with 5.2.1, the overlay never shows, see this console error, which looks like the new code added:

overlay.js:602 Uncaught TypeError: Cannot read properties of null (reading 'stack') at eval (overlay.js:602:1)

Once again, we don't use react. Just plain, plain as plain vanilla JS.

im having this problem currently, all typescript and react

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: don't show error overlay when receiving error that was caught by React error boundary
4 participants