Skip to content

feat(wrangler): improve script source display on pretty error screen - #9971

Merged
edmundhung merged 3 commits into
mainfrom
edmundhung/DEVX-1974
Jul 23, 2025
Merged

feat(wrangler): improve script source display on pretty error screen#9971
edmundhung merged 3 commits into
mainfrom
edmundhung/DEVX-1974

Conversation

@edmundhung

@edmundhung edmundhung commented Jul 15, 2025

Copy link
Copy Markdown
Member

Fixes DEVX-1974

Example screenshots:

  • Error from native module (Requrie "View All Frames" checked):

    Screenshot 2025-07-21 at 13 24 52
  • Error from missing source file (e.g. cookie)

    Screenshot 2025-07-16 at 12 55 41

  • Tests
    • Tests included
    • Tests not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: error screen is not documented
  • Wrangler V3 Backport
    • Wrangler PR:
    • Not necessary because: not bugfix

@changeset-bot

changeset-bot Bot commented Jul 15, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ae2bbd6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
wrangler Minor
miniflare Minor
@cloudflare/vite-plugin Major
@cloudflare/vitest-pool-workers Patch
@cloudflare/pages-shared Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jul 15, 2025

Copy link
Copy Markdown
create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9971

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9971

miniflare

npm i https://pkg.pr.new/miniflare@9971

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9971

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9971

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9971

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9971

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9971

wrangler

npm i https://pkg.pr.new/wrangler@9971

commit: ae2bbd6

@edmundhung
edmundhung force-pushed the edmundhung/DEVX-1974 branch from b690074 to 016388b Compare July 16, 2025 10:26
@edmundhung edmundhung changed the title feat: improve source display on pretty error page feat(wrangler): improve script source display on pretty error screen Jul 16, 2025
@edmundhung
edmundhung force-pushed the edmundhung/DEVX-1974 branch from 016388b to f0007b3 Compare July 16, 2025 11:28
@edmundhung
edmundhung force-pushed the edmundhung/DEVX-1974 branch from f0007b3 to e04e371 Compare July 16, 2025 11:39

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We are now relying on the UserWorker miniflare instance to prettify the error so that we can retrieve the script source from its inspector.

Comment on lines 128 to 130

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We will continue stripping this header by default, except on the ProxyWorker miniflare which will pass the header over to the UserWorker miniflare so that user can disable the pretty error screen when needed.

Comment thread patches/youch@4.1.0-beta.10.patch Outdated
Comment on lines 9 to 10

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Youch currently skip displaying the script source if it is an internal module. But we do want it to show up if frame.source is defined. I will put up a PR to youch later.

Comment thread patches/youch@4.1.0-beta.10.patch Outdated
return "";
}
- const language = LANGS_MAP[extname(frame.fileName)] ?? "plain";
+ const language = LANGS_MAP[extname(frame.fileName)] ?? (frame.type === "native" ? "js" : "plain");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Native modules have no extension so there is no syntax highlight by default. But it should be safe to assume native modules to be javascript if available?

Comment thread patches/youch@4.1.0-beta.10.patch Outdated
#getEditorLink(ide, frame) {
const editorURL = EDITORS[ide] || ide;
- if (!editorURL || frame.type === "native") {
+ if (!editorURL || frame.type === "native" || !existsSync(frame.fileName)) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure if this is a good patch upstream as it assumes youch is running on node environment only. But I will raise it up when I submit a PR to youch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this optional so it can run in non-node environments? We run Youch ourselves in a non-node environment (format-errors), so it would be nice to make that possible without patches (that we apply to format-errors)

@edmundhung
edmundhung marked this pull request as ready for review July 16, 2025 11:58
@edmundhung
edmundhung requested a review from a team as a code owner July 16, 2025 11:58
@edmundhung
edmundhung requested a review from penalosa July 16, 2025 11:58
Comment thread packages/miniflare/src/plugins/core/errors/index.ts Outdated
Comment thread packages/miniflare/src/plugins/core/errors/index.ts Outdated
Comment thread packages/miniflare/src/plugins/core/errors/index.ts Outdated
Comment thread patches/youch@4.1.0-beta.10.patch Outdated
#getEditorLink(ide, frame) {
const editorURL = EDITORS[ide] || ide;
- if (!editorURL || frame.type === "native") {
+ if (!editorURL || frame.type === "native" || !existsSync(frame.fileName)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this optional so it can run in non-node environments? We run Youch ourselves in a non-node environment (format-errors), so it would be nice to make that possible without patches (that we apply to format-errors)

#getEditorLink(ide, frame) {
const editorURL = EDITORS[ide] || ide;
- if (!editorURL || frame.type === "native") {
+ if (!editorURL || frame.type === "native" || frame.fileType !== 'fs') {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We are now relying on the fileType to decide whether Youch should generate a link.

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jul 23, 2025
@edmundhung
edmundhung merged commit 19794bf into main Jul 23, 2025
48 of 50 checks passed
@edmundhung
edmundhung deleted the edmundhung/DEVX-1974 branch July 23, 2025 12:50
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants