Skip to content

lib: expose global ErrorEvent #58920

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Richienb
Copy link
Contributor

@Richienb Richienb commented Jul 1, 2025

Fixes #58918

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup
  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 1, 2025
@panva panva added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 1, 2025
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.08%. Comparing base (a7a37c3) to head (1b4cf60).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58920      +/-   ##
==========================================
- Coverage   90.11%   90.08%   -0.03%     
==========================================
  Files         640      640              
  Lines      188427   188473      +46     
  Branches    36956    36971      +15     
==========================================
- Hits       169792   169777      -15     
- Misses      11348    11423      +75     
+ Partials     7287     7273      -14     
Files with missing lines Coverage Δ
...internal/bootstrap/web/exposed-window-or-worker.js 93.79% <100.00%> (ø)

... and 37 files with indirect coverage changes

🚀 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.

@panva panva added the notable-change PRs with changes that should be highlighted in changelogs. label Jul 1, 2025
Copy link
Contributor

github-actions bot commented Jul 1, 2025

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @panva.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@himself65 himself65 added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 2, 2025
@nodejs-github-bot
Copy link
Collaborator

@panva
Copy link
Member

panva commented Jul 2, 2025

cc @nodejs/tsc for semver-major PRs that contain breaking changes and should be released in the next major version.

@panva
Copy link
Member

panva commented Jul 2, 2025

@KhafraDev Is the class is tested in https://github.com/nodejs/undici inclusive all its related WPTs?

Are there ErrorEvent WPTs we could enable in https://github.com/nodejs/node?

Copy link
Member

@panva panva left a comment

Choose a reason for hiding this comment

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

Some form of test coverage should be present here.

@KhafraDev
Copy link
Member

@KhafraDev Is the class is tested in https://github.com/nodejs/undici inclusive all its related WPTs?

Are there ErrorEvent WPTs we could enable in https://github.com/nodejs/node?

It is tested, but all of the WPTs iirc were html files, so I adapted them to run in our own test suite. There isn't anything node needs to enable. It should be safe to assume that, similar to everything else from undici, it follows the spec even closer than pretty much anything in node core.

https://github.com/nodejs/undici/blob/main/test/websocket/events.js

@Richienb Richienb requested a review from aduh95 July 3, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ErrorEvent
7 participants