fix: preserve NonRetryableError message when compat flag is enabled#13560
fix: preserve NonRetryableError message when compat flag is enabled#13560vaishnav-mk wants to merge 4 commits intocloudflare:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 9c066a0 The changes in this PR will be included in the next version bump. 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 |
workers-devprod
requested review from
a team and
penalosa
and removed request for
a team
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
vaishnav-mk
force-pushed
the
vaish/fix-nonretryable-error-message
branch
from
cb2fa30 to
8b56879
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
vaishnav-mk
force-pushed
the
vaish/fix-nonretryable-error-message
branch
from
8b56879 to
396be46
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
vaishnav-mk
force-pushed
the
vaish/fix-nonretryable-error-message
branch
2 times, most recently
from
71252fa to
0cbbdff
Compare
petebacondarwin
left a comment
petebacondarwin
left a comment
There was a problem hiding this comment.
Looking much cleaner. It's a shame that one must pass through the compat flags but I can't think of a more straightforward approach.
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
vaishnav-mk
force-pushed
the
vaish/fix-nonretryable-error-message
branch
from
0cbbdff to
965ee4f
Compare
| message: "my custom error message", | ||
| }); | ||
|
|
||
| vi.unstubAllGlobals(); |
There was a problem hiding this comment.
🟡 Stubbed global Cloudflare leaks to subsequent tests if test fails
The test at packages/workflows-shared/tests/engine.test.ts:69 stubs globalThis.Cloudflare via vi.stubGlobal, but vi.unstubAllGlobals() is only called at line 128 at the end of the test body. If any assertion or await between lines 69-127 throws, the stub is never cleaned up. The afterEach hook (line 18) only calls workerdUnsafe.abortAllDurableObjects() — it does not unstub globals. The shared vitest config's restoreMocks: true only restores spies (.mockRestore()), not globals stubbed with vi.stubGlobal. This means subsequent tests would see a fake Cloudflare global with only workflows_preserve_non_retryable_error_message set, causing shouldPreserveNonRetryableError() to return true unexpectedly and potentially masking test failures or causing false passes.
Prompt for agents
The vi.unstubAllGlobals() call at the end of the test body (line 128) will not execute if the test throws before reaching it. Move the cleanup into an afterEach or use a try/finally block to ensure the Cloudflare global stub is always cleaned up.
Option 1: Add vi.unstubAllGlobals() to the existing afterEach hook at line 18-20.
Option 2: Wrap the test body in a try/finally with vi.unstubAllGlobals() in the finally block.
Option 3: Add unstubGlobals: true to the vitest config for this package.
Option 1 is simplest and mirrors the existing afterEach pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Fixes the NonRetryableError message being replaced with a generic error message in local dev
When the workflows_preserve_non_retryable_error_message compatibility flag is enabled, NonRetryableError now preserves the original error name and message instead of replacing it with a generic WorkflowFatalError message
workflows_preserve_non_retryable_error_message). Docs will be added when it graduates to a compat date.A picture of a cute animal (not mandatory, but encouraged)
