Fix/Improve processWindowErrorEvent

[silverwind]

• e.error can be undefined in some cases which would raise an error inside this error handler, fixed that.
• The displayed message mentions looking into the console, but in my case of error from ResizeObserver there was nothing there, so add this logging. I think this logging was once there but got lost during refactoring.

[silverwind]

Maybe browser does the logging, but only if there is e.error, need to test.

[silverwind]

Putting draft until I have confirmed above behaviour. Definitely should backport this, otherwise some "errors" will cause errors in inside this error handler.

[silverwind]

Rewrote the function, now it truly handles all cases and the comments accurately describe what it does.

Flashes:

Console:

[silverwind]

As per mozilla-mobile/firefox-ios#10817 (comment), the IOS Firefox issue classifies into the !err category, so it's an error event, not an error and we can use the same handler that catches the ResizeObserver events. I added some links to the comment above it and made it not show any error event messages in production.

[silverwind]

I don't like this change. Because "e" could be anything by design.

But I won't block it.

I know you don't like such destructuring, but ErrorEvent and its properties are well defined, including in the HTML standard, so I see no issue.

[silverwind]

The handler also receives PromiseRejectionEvent and I see its reason property is not guaranteed to be an Error, so it could be improved to handle that case.

[wxiaoguang]

I know you don't like such destructuring

Not really. The real problem is Because "e" could be anything by design.. Before the old code (before #29303) was quite straightforward: if this error, do this, if that error, do that. But the new code just mixed everything together. I don't like making readers "guess" what the type/value there should be.

[wxiaoguang]

ps: the last time I said "no" to such syntax, the reason is the same: it make code unclear (#25253 (comment)), and introduced new bugs #25253 (comment)

[silverwind]

I can restore the previous type check, no problem.

[silverwind]

I decided against type check as there is no overlap in properties between ErrorEvent and PromiseRejectionEvent, so it would unnecessarily bloat the code. I fixed the jsDoc, added a typecheck for Error before accessing stack, and tweaked the comment again.

Everything works as expected:

[silverwind]

Before the old code (before #29303) was quite straightforward: if this error, do this, if that error, do that. But the new code just mixed everything together. I don't like making readers "guess" what the type/value there should be.

It's pretty straightforward already. Could be made even more so by splitting into two handler functions hat converge into a logging function, but I think would be in overengineering territory. I don't see the current function as hard to read just because of this error ?? reason, both properties serve the same purpose.

[silverwind]

@delvh @yardenshoham maybe you want to give this another look as it has changed a bit since your reviews. If there no further objections, I will merge this soon.

[yardenshoham]

Please recommend ways to test

[silverwind]

Can be tested by installing Violentmonkey with such userscripts:

// ==UserScript==
// @name        Gitea error trigger 1
// @match       http://localhost:3000/*
// ==/UserScript==
setTimeout(() => {
  throw new Error("sync error from userscript");
}, 10000)

const divElem = document.querySelector(".repo-header");
const resizeObserver = new ResizeObserver((entries) => {
  for (const entry of entries) {
    entry.target.style.width = entry.contentBoxSize[0].inlineSize + 10 + "px";
  }
});
resizeObserver.observe(divElem);

// ==UserScript==
// @name        Gitea error trigger 2
// @match       http://localhost:3000/*
// ==/UserScript==
(async () => {
  throw new Error("async error from userscript");
})()

[yardenshoham]

I can confirm I see the errors

[silverwind]

If you go on a repo page, you should also see the proper ResizeObserver error :)