Skip to content

[Tests] Cover theme dev error page rendering and HTML escaping - #8551

Merged
gonzaloriestra merged 1 commit into
mainfrom
tests-maintenance-34913123511
Sep 16, 2026
Merged

gonzaloriestra merged 1 commit into
mainfrom
tests-maintenance-34913123511

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

getErrorPage renders the HTML page shown when theme dev fails to render a page, and it had no test coverage. Its escapeHtml helper is the only thing preventing untrusted error text (Liquid error messages, stack traces, file paths from the theme) from being injected as markup into that page, and nothing guarded that behavior against regression.

WHAT is this pull request doing?

Adds error-page.test.ts next to error-page.ts, covering the public contract of getErrorPage:

  • The title and header appear in the rendered page.
  • Every error's message and code is rendered, not just the first.
  • HTML in error messages and codes is escaped, so a <script> tag in an error cannot inject markup.
  • A complete document is still returned when the error list is empty.

Removing the escapeHtml calls from error-page.ts makes the escaping test fail, so the test earns its keep. No production code is changed.

How to manually test your changes?

CI

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@Suleimanlatrsh Suleimanlatrsh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested locally, including HTML in the error messages and code. Looks good 👍

@gonzaloriestra
gonzaloriestra marked this pull request as ready for review September 16, 2026 08:56
@gonzaloriestra
gonzaloriestra requested a review from a team as a code owner September 16, 2026 08:56
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Sep 16, 2026
@gonzaloriestra
gonzaloriestra added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit b73f487 Sep 16, 2026
27 of 29 checks passed
@gonzaloriestra
gonzaloriestra deleted the tests-maintenance-34913123511 branch September 16, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants