Fix `relative-time` error and improve global error handler by silverwind · Pull Request #37241 · go-gitea/gitea

silverwind · 2026-04-16T17:46:25Z

Also enhances the global error UI per the issue's second half: the error message doubles as a <summary> that expands to show the stack trace (<pre><code>) when clicked. The whole error banner is clickable, so users can easily copy-paste the stack into bug reports. Drops the "Open browser console to see more details." hint.

Adds a generic e2e test that asserts no JS error on the homepage.

This PR was written with the help of Claude Opus 4.7

`navigator.language` can be the string `"undefined"` in Playwright Firefox (not just the undefined value), causing `Intl.DateTimeFormat` to throw `RangeError: invalid language tag: "undefined"`. The prior fix in go-gitea#37021 only handled the undefined value. Validate both the `lang` attribute and `navigator.language` through `Intl.Locale` and fall back to `'en'` if both yield invalid tags. Also enhances the global error UI: - Render the error stack trace (hidden, included when copying) - Add a copy-to-clipboard button with icon-swap feedback - Drop the "Open browser console" hint - Expose `copy` i18n key for the button's aria-label Fixes go-gitea#37239 Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Rename `showGlobalErrorMessage` to `showGlobalError`, accept an `Error` as the first argument, and move the `"JavaScript error:"` / `"JavaScript promise rejection:"` prefix into the function. Replace raw `navigator.clipboard.writeText` with `clippie` and guard rapid clicks from stacking reset timeouts. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Copilot

Pull request overview

Fixes a Playwright/headless-only relative-time locale parsing crash and enhances the frontend global error handler to better capture/copy debugging details, plus adds an e2e smoke test to catch homepage JS errors.

Changes:

Make relative-time locale resolution resilient to invalid navigator.language values and add a regression unit test.
Replace showGlobalErrorMessage with showGlobalError(Error, opts) and extend the global error UI with a copy-to-clipboard button that includes stack traces.
Add a Playwright test intended to ensure the homepage renders without JS errors.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
web_src/js/webcomponents/relative-time.ts	Safer language fallback when locale candidates are invalid.
web_src/js/webcomponents/relative-time.test.ts	Adds regression test for invalid `navigator.language`.
web_src/js/modules/errors.ts	New `showGlobalError` API + copy-with-stack UI + updated error event processing.
web_src/js/modules/errors.test.ts	Updates/extends unit tests for new error handler behavior.
web_src/js/features/common-page.ts	Migrates warning banners to the new `showGlobalError` API.
web_src/js/bootstrap.ts	Uses `showGlobalError` for early bootstrap failures.
tests/e2e/homepage.test.ts	Adds homepage smoke test asserting no global error UI is rendered.
templates/base/head_script.tmpl	Adds `i18n.copy` string to `window.config` for the new copy button label.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Keep the public signature as `showGlobalErrorMessage(msg, msgType?, stack?)` with a new third `stack` param. Minimizes the diff against main: bootstrap.ts and common-page.ts revert to their original call shape; only `processWindowErrorEvent` passes the new `stack` arg. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

- Drop the copy button's aria-label (and the newly-added i18n.copy key) so the error UI has no runtime dependency on window.config and works even when bootstrap triggers showGlobalErrorMessage due to a missing window.config. - Preserve newlines in multi-line messages (e.g. the ROOT_URL warning) via tw-whitespace-pre-line on .js-global-error-msg. - Always overwrite .js-global-error-stack so a previously-set stack doesn't linger when a later call has none. - Add networkidle wait to the homepage e2e so async JS errors have time to reach the global handler before the assertion; disable playwright/no-networkidle globally for e2e tests. - Simplify tests: shared beforeEach, drop redundant DOM-structure assertions, remove unneeded teardown plumbing. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Coverage is already provided by the `processWindowErrorEvent renders stack trace` test. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Keep comments and `.toString()` choices consistent with main so the diff focuses on the actual behavioral changes. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

wxiaoguang

No, the error message prompt is just for development purpose.

It should not be that complex. The more complex it becomes, the more potential bugs it will have. What if the code causes new errors again?

Do not make things unnecessarily complex.

If you need to read more about the error details, just open the browser's console, just like all the frontend developers do.

silverwind · 2026-04-17T08:06:38Z

Stack trace must be accessible somehow because it's vital information and a small copy button achieves that in the least obtrusive way besides rendering it directly.

"Open the console" is bad, some browsers might not even have a console, think about tablets or phones. Don't assume every user is on a desktop.

wxiaoguang · 2026-04-17T08:09:53Z

Stack trace must be accessible somehow because it's vital information and a small copy button achieves that in the least obtrusive way besides rendering it directly.

Do you remember that you had objection when I introduced such "js error message prompt"? But now why you keep making it unnecessarily complex. Developers should open their console, look into the real source code with source map.

In production build, your stack trace here is just garbled text

"Open the console" is bad, some browsers might not even have a console, think about tablets or phones. Don't assume every user is on a desktop.

It is just your guess and imagination that "it needs to show stack for a user who uses a phone".

silverwind · 2026-04-17T08:12:29Z

It's more about that I always have to ask users to copy the error from the console than to screenshot/copy only the message because "Foo is undefined" is not debuggable without the stack trace. If you don't like the copy button, we could make it visible on-hover.

wxiaoguang · 2026-04-17T08:14:42Z

It's more about that I always have to ask users to copy the error from the console

How many times? Where are the related issues?

If you don't like the copy button, we could make it visible on-hover.

I don't accept any more useless change.

silverwind · 2026-04-17T08:17:43Z

How many times? Where are the related issues?

At least 4-5 times, some on GitHub, some on Discord.

I don't accept any more useless change.

It is genuinely needed for debugging.

silverwind · 2026-04-17T08:29:41Z

Concrete example:

User posts only a screenshot of the error without any console output. Errors was deducable from the message via AI, but in case of e is undefined, this won't be possible.

silverwind · 2026-04-17T08:33:01Z

If copy button is not to your liking, we can use a <details>+<summary> that expands with the stack track on click and maybe add a "Click for details" in place of "Open browser console". Least obtrusive change possible.

wxiaoguang · 2026-04-17T09:49:13Z

Concrete example:

User posts only a screenshot of the error without any console output. Errors was deducable from the message via AI, but in case of e is undefined, this won't be possible.

They are contributors, they should know how to correctly report a bug without the "copy" button or duplicate "stack" message.
Most end users should never see the JS error, most JS errors should already have been handled by maintainers during the development.
Even if you have a "copy" button, almost all users still only show you a screenshot (just like your screenshot shows), you still need to tell them "please provide the stacktrace", the same as "open the console and show the stacktrace"

wxiaoguang · 2026-04-17T09:50:29Z

If copy button is not to your liking, we can use a <details>+<summary> that expands with the stack track on click and maybe add a "Click for details" in place of "Open browser console". Least obtrusive change possible.

Can do with a general function like

export function showGlobalErrorMessage(msg: string, msgType: Intent = 'error', details?: string) {
...
}

silverwind · 2026-04-17T10:08:35Z

I will do the details+summary approach like that.

Per review feedback (wxiaoguang), drop the copy button + explicit stack pre in favor of a native <details> expander: the message itself is the summary, clicking anywhere on the banner toggles the stack trace, and the stack renders in <pre><code> centered as a block with left-aligned text. Keeps the no-stack path identical to main. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-17T22:53:35Z

Ready again. now uses <details>, diff is minimal. PR description updated with new screenshots.

wxiaoguang · 2026-04-18T03:25:16Z

+  await page.goto('/');
+  await page.waitForLoadState('networkidle'); // eslint-disable-line playwright/no-networkidle
+  await assertNoJsError(page);
+});


Why such a strange special test for "homepage"? No page should have JS error

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 16, 2026

github-actions bot added modifies/templates This PR modifies the template files modifies/frontend labels Apr 16, 2026

silverwind changed the title ~~Fix relative-time error with invalid navigator.language~~ Fix relative-time error and improve global error handler Apr 16, 2026

silverwind and others added 2 commits April 16, 2026 19:53

Add margin and green tint to copy-success feedback

6bba33a

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind requested a review from Copilot April 16, 2026 18:21

Copilot started reviewing on behalf of silverwind April 16, 2026 18:22 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread web_src/js/modules/errors.ts Outdated

Comment thread web_src/js/modules/errors.ts Outdated

Comment thread web_src/js/modules/errors.ts Outdated

Comment thread web_src/js/modules/errors.ts Outdated

Comment thread tests/e2e/homepage.test.ts

silverwind and others added 2 commits April 16, 2026 20:35

github-actions bot added topic/code-linting and removed modifies/templates This PR modifies the template files labels Apr 16, 2026

Drop redundant stack-for-copy test

fed4ff3

Coverage is already provided by the `processWindowErrorEvent renders stack trace` test. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind commented Apr 16, 2026

View reviewed changes

Comment thread web_src/js/modules/errors.ts Outdated

Shrink diff against main

7d99c9d

Keep comments and `.toString()` choices consistent with main so the diff focuses on the actual behavioral changes. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

wxiaoguang requested changes Apr 17, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 17, 2026

silverwind marked this pull request as draft April 17, 2026 10:08

github-actions bot removed the topic/code-linting label Apr 17, 2026

Merge branch 'main' into relative-time-invalid-lang

708ec77

silverwind marked this pull request as ready for review April 17, 2026 22:53

wxiaoguang reviewed Apr 18, 2026

View reviewed changes

Uh oh!

Conversation

silverwind commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wxiaoguang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

silverwind commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 17, 2026

Uh oh!

silverwind commented Apr 17, 2026

Uh oh!

silverwind commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 17, 2026

Uh oh!

wxiaoguang commented Apr 17, 2026

Uh oh!

silverwind commented Apr 17, 2026

Uh oh!

silverwind commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

silverwind commented Apr 16, 2026 •

edited

Loading

wxiaoguang left a comment •

edited

Loading

silverwind commented Apr 17, 2026 •

edited

Loading

wxiaoguang commented Apr 17, 2026 •

edited

Loading

silverwind commented Apr 17, 2026 •

edited

Loading

silverwind commented Apr 17, 2026 •

edited

Loading

silverwind commented Apr 17, 2026 •

edited

Loading

silverwind commented Apr 17, 2026 •

edited

Loading