Skip to content

[format-errors] Await async handlePrettyErrorRequest in fetch handler's try/catch#12756

Draft
petebacondarwin wants to merge 2 commits intomainfrom
devin/1772632824-fix-format-errors-missing-await
Draft

[format-errors] Await async handlePrettyErrorRequest in fetch handler's try/catch#12756
petebacondarwin wants to merge 2 commits intomainfrom
devin/1772632824-fix-format-errors-missing-await

Conversation

@petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Mar 4, 2026

Fixes #12755.

handlePrettyErrorRequest is an async function returning Promise<Response>, but it was returned without await in the fetch handler's try/catch block. If the promise rejects (e.g. Youch throws asynchronously), the catch block never executes — the error counter doesn't increment, Sentry doesn't capture the exception, and the user sees an unhandled rejection instead of the 500 JSON fallback.

The fix is a one-character change: return handlePrettyErrorRequest(payload)return await handlePrettyErrorRequest(payload).

This PR also adds the first unit tests for this package (for both handlePrettyErrorRequest and reviveError), along with the necessary vitest configuration.

Reviewer notes:

  • The vitest.config.mts includes a resolve.alias for promjs because the installed package has a broken main field (lib/index.js doesn't exist; the actual entry is index.js in the package root). This alias lets Vite resolve it in tests.
  • vitest is not explicitly listed as a devDependency of format-errors — it resolves from the workspace root. Worth considering whether to add it explicitly.
  • No changeset is included because @cloudflare/format-errors is a deployed Worker — changesets for it are disallowed in CI as they would trigger a deployment.

Suggested review focus:

  • Verify the promjs alias approach is acceptable vs. alternative fixes.
  • The test for the await fix (handlePrettyErrorRequest test) uses vi.doMock + dynamic import() to get a fresh module with a mocked Youch that rejects — confirm this correctly proves the bug scenario.

Devin PR requested by @petebacondarwin
Link to Devin session: https://app.devin.ai/sessions/3ad7146985384db895a3cb4a235cd1bf


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this is an internal bug fix with no user-facing API changes

…'s try/catch

Add missing await on the async handlePrettyErrorRequest() call so that
promise rejections are properly caught by the surrounding try/catch block.
Without await, async errors would result in unhandled rejections instead of
being caught — meaning the error counter wouldn't increment, Sentry wouldn't
capture the exception, and users wouldn't receive the 500 JSON fallback.

Also adds unit tests for handlePrettyErrorRequest and reviveError.

Fixes #12755

Co-Authored-By: pbacondarwin <pete@bacondarwin.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@changeset-bot
Copy link

changeset-bot bot commented Mar 4, 2026

⚠️ No Changeset found

Latest commit: 66a98ea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

✅ All changesets look good

@cloudflare/format-errors is a deployed Worker and changesets for it are
not allowed in CI since they would trigger a deployment.

Co-Authored-By: pbacondarwin <pete@bacondarwin.com>
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 4, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: fcc867f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

missing await on async function in format-errors catch block

1 participant