Cosmetic consistency pass on remaining bare throws#1838
Cosmetic consistency pass on remaining bare throws#1838pranaygp wants to merge 2 commits intopranaygp/friendlier-errors-phase-5-attributionfrom
Conversation
🦋 Changeset detectedLatest commit: d8b41c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
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 |
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests💻 Local Development (2 failed)vite-stable (2 failed):
Details by Category❌ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
There was a problem hiding this comment.
Pull request overview
This PR continues the “friendlier-errors” series by replacing a few remaining bare throw new Error(...) sites in @workflow/core with more structured WorkflowRuntimeError throws and improving schema-validation error readability in defineHook().resume().
Changes:
- Converted several internal invariant/VM-constraint error throws to
WorkflowRuntimeErrorfor consistent SDK attribution viadescribeError. - Improved
defineHook().resume()schema validation failures to render as a readable multi-line list rather than a JSON dump. - Updated unit tests/snapshots to match the new error messages.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/workflow.ts | Uses WorkflowRuntimeError for missing startedAt invariant. |
| packages/core/src/vm/index.ts | Throws WorkflowRuntimeError with actionable guidance for crypto.subtle.generateKey() in the workflow VM. |
| packages/core/src/vm/index.test.ts | Updates assertions to match the new generateKey() error message. |
| packages/core/src/step/get-closure-vars.ts | Uses WorkflowRuntimeError when closure vars are accessed outside step context. |
| packages/core/src/global.ts | Makes ENOTSUP() throw WorkflowRuntimeError with clearer VM/step guidance. |
| packages/core/src/define-hook.ts | Formats schema validation issues into a readable list before throwing. |
| packages/core/src/define-hook.test.ts | Updates inline snapshot for the new formatted validation error. |
| .changeset/friendlier-errors-consistency.md | Adds a patch changeset describing the consistency/error-message changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Stacked: #1839 — Phase 7 foundation, data-driven describeRunError + public subpath |
eb171bb to
3dd68cc
Compare
Internal invariants now use WorkflowRuntimeError so describeError attributes them to the SDK: missing startedAt, VM generateKey, closure-vars outside step context, ENOTSUP. defineHook().resume() formats schema validation failures as a readable list instead of a JSON blob. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
f15790b to
d8b41c0
Compare
|
Superseded by #1849 — consolidated friendlier-errors PR with all 8 phases + follow-up fixes (ANSI leak, non-retry semantics, shared captureStackTrace helper). |
Summary
Phase 6 of the friendlier-errors stack. A small consistency pass converting the last ergonomically-important bare
throw new Error(...)sites in@workflow/coreto structured error classes and friendlier messages.WorkflowRuntimeErrorsodescribeErrorattributes them to the SDK, not the user:workflow.ts: missingstartedAttimestamp (should not happen)vm/index.ts:crypto.subtle.generateKey()disabled inside the workflow VM — now explains why and points to step functionsstep/get-closure-vars.ts: closure-vars accessed outside a step contextglobal.ts(ENOTSUP): explains the workflow-VM constraint and suggests moving the call to a step functiondefineHook().resume(): schema validation failures are rendered as a readable list (at "field": <message>) instead of a raw JSON dump.Manual test plan
Using
workbench/nextjs-turbopack—pnpm devand watch the terminal.Zod schema failure on
defineHook().resume()— the headline user-visible change:Call
paymentHook.resume(token, { amount: -5 })from a server route. Expect a readable bulleted list of validation issues — one line per issue with the field path and message (at "amount": Number must be greater than 0) — NOT a raw JSON dump ofZodError.issues.crypto.subtle.generateKey()inside workflow VM — call it from a"use workflow"function:Expect a clear message explaining why it's disabled in the VM and pointing at "move this into a step function".
WorkflowRuntimeError→'sdk'attribution — reusing Phase 5 behavior: confirm the log for the above test includeserrorAttribution: 'sdk'(not'user'). Internal invariants should be framed as "this is us, not you."Non-Zod schema (e.g. valibot) — if you use a non-Zod schema validator, confirm the list rendering still works via the shared error-shaping path.
Rare internal invariants — these only trip on severe bugs; not generally testable by hand. Covered by unit tests.
Unit tests
pnpm --filter @workflow/core exec vitest run src/define-hook.test.ts src/vm/index.test.ts src/describe-error.test.tsDOMException/world.streamsfailures still match baseline📚 Friendlier errors stack
Multi-PR initiative inspired by @Schniz's stalled #706:
Ansirendering primitives + context-violation errorsSerializationErrorat serialization / stream / encryption boundariesdescribeError)throw new Error(...)sitesdescribeRunError+ public subpathWorkflowBuildError+ applications in@workflow/buildersfunctionNameleak, simplify docs framing, redirect stack to user codeEach PR is stacked on the previous one; merge in order.
🤖 Generated with Claude Code