-
Notifications
You must be signed in to change notification settings - Fork 245
Friendlier workflow errors (consolidated) #1849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
pranaygp
wants to merge
25
commits into
main
Choose a base branch
from
pranaygp/friendlier-errors-followups
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
aef3ef3
Introduce structured context-violation errors + Ansi renderer
pranaygp cec8cfe
Address review: tighten changeset, implement ansifyName, harden Ansi
pranaygp dff52c9
Structured runtime logger metadata + fold in replay-timeout logging
pranaygp b2b1587
Use double-quoted changeset frontmatter per repo convention
pranaygp 25391ce
Add SerializationError + apply to user-facing serialization sites
pranaygp 351d330
Use double-quoted changeset frontmatter per repo convention
pranaygp 60b90d0
Presentation-only user vs SDK error attribution
pranaygp 3dd68cc
Address review: describeError accepts precomputed errorCode + instanceof
pranaygp e2cb99f
Cosmetic consistency pass on remaining bare throws
pranaygp d8b41c0
Use double-quoted changeset frontmatter per repo convention
pranaygp e6b8e31
Data-driven describeRunError + expose via @workflow/core/describe-error
pranaygp efaba3a
Friendlier build-time errors: WorkflowBuildError class + applications
pranaygp d9eb4f5
Polish friendlier-errors rendering: drop functionName leak, simplify …
pranaygp 107da09
Consolidate friendlier-errors stack: fix ANSI leak + non-retry semantics
pranaygp 85268a9
test: update step-handler mocks for scoped forRun() logger
pranaygp d37bd23
Mark SerializationError fatal + route dehydration through step-failur…
pranaygp aac6526
Add logging snapshot tests + manual-test artifacts
pranaygp fd77555
Readable step-fatal logs: inline stack + friendly step/workflow names
pranaygp 9fd914b
Opinionated pretty formatter for runtime structured-log metadata
pranaygp 678383b
Merge main into pranaygp/friendlier-errors-followups
pranaygp 6b6a1cd
Merge remote-tracking branch 'origin-https/main' into pranaygp/friend…
pranaygp 7774978
ci(benchmarks): disable pnpm cache for getCommunityWorldsMatrix
pranaygp d900ccd
Merge branch 'main' into pranaygp/friendlier-errors-followups
pranaygp 9d45cdf
Address PR review comments: inspect dedup, cause leak, retry-loop tests
pranaygp 351971a
Consolidate changesets + remove pr-artifacts
pranaygp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| "@workflow/core": patch | ||
| "@workflow/errors": patch | ||
| "@workflow/builders": patch | ||
| "@workflow/utils": patch | ||
| --- | ||
|
|
||
| Friendlier workflow error messages. New `SerializationError`, `WorkflowBuildError`, and structured context-violation classes (e.g. `NotInWorkflowContextError`) with actionable hints and docs links applied to user-facing throw sites; `FatalError.is()` recognizes any error with `fatal: true` so context violations and serialization failures now fail fast instead of burning retry attempts. Runtime logs are namespaced under `[workflow-sdk]` and gain `errorAttribution` (`user` vs `sdk`) plus class-aware hints; `Ansi` helpers moved to a new `@workflow/errors/ansi` subpath so consumers that only use the error classes don't pull `chalk` into their bundle. Adds a `@workflow/core/describe-error` subpath so CLI / web observability renderers can derive the same user-vs-SDK framing from persisted failure events. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,16 @@ | ||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||
| "@workflow/core": patch | ||||||||||||||||||||||||||
| "@workflow/errors": patch | ||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Replace `util.inspect`'s default object dump for runtime structured-log metadata with an opinionated, workflow-aware formatter (`packages/core/src/log-format.ts`). The runtime logger now composes `[workflow-sdk] <message>` + stack + a compact, color-coded metadata block — passed to `console.error` / `console.warn` as a single string — instead of letting Node quote-escape multi-line stacks and paragraph hints inside an object dump. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Highlights of the new format: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - `wrun_…` / `step_…` ULIDs render with their parsed friendly name (`add (./workflows/1_simple)`) using the existing `parseStepName` / `parseWorkflowName` utilities. | ||||||||||||||||||||||||||
| - Color-coded attribution badge (`user error` red, `sdk error` magenta) paired with the error class in bold. | ||||||||||||||||||||||||||
| - `hint` renders as a clean paragraph under `hint:` instead of a backslash-`\n`-escaped string. | ||||||||||||||||||||||||||
| - Redundant fields (`errorStack`, plus `errorMessage` when the parent message already includes it) are dropped to avoid double-printing. | ||||||||||||||||||||||||||
| - Unknown fields fall through as a sorted `key value` tail so we never silently drop log information. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Side-effect: `@workflow/errors/ansi` gains `bold`, `red`, `magenta` helpers used by the formatter. The `web` / `web-shared` packages don't consume stderr — they read structured event payloads from the World event log — so the change is presentation-only at the runtime layer. | ||||||||||||||||||||||||||
|
Comment on lines
+6
to
+16
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /** | ||
| * V8-only (Node, Bun, Chrome, Deno). Rewrites `err.stack` so the top frame is | ||
| * the caller of `stackStartFn` instead of the framework function that threw. | ||
| * Without this, terminal overlays (Next.js, Turbopack, VS Code) render the | ||
| * code frame at our `throw` site inside `@workflow/core`, which is useless | ||
| * to the user. | ||
| * | ||
| * No-op on engines that don't expose `Error.captureStackTrace` — the stack | ||
| * degrades gracefully to the default behavior. | ||
| * | ||
| * Kept in its own tiny module so callers that can't participate in the | ||
| * `context-errors.ts` ↔ `workflow/get-workflow-metadata.ts` import cycle can | ||
| * still pull in the helper without pulling in the full error classes. | ||
| */ | ||
| export function redirectStackToCaller( | ||
| err: Error, | ||
| // biome-ignore lint/complexity/noBannedTypes: signature matches Error.captureStackTrace | ||
| stackStartFn: Function | ||
| ): void { | ||
| const capture = ( | ||
| Error as unknown as { | ||
| captureStackTrace?: (target: object, fn: Function) => void; | ||
| } | ||
| ).captureStackTrace; | ||
| capture?.(err, stackStartFn); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,242 @@ | ||
| import { FatalError } from '@workflow/errors'; | ||
| import { inspect } from 'node:util'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
| import { | ||
| NotInStepContextError, | ||
| NotInWorkflowContextError, | ||
| NotInWorkflowOrStepContextError, | ||
| throwNotInWorkflowContext, | ||
| UnavailableInWorkflowContextError, | ||
| } from './context-errors.js'; | ||
| import { | ||
| WORKFLOW_CONTEXT_SYMBOL, | ||
| type WorkflowMetadata, | ||
| } from './workflow/get-workflow-metadata.js'; | ||
|
|
||
| // These tests assert on the plain-text form of the messages. In a TTY chalk | ||
| // would add color, but vitest runs without a TTY so chalk is level=0 and | ||
| // the styling helpers are pass-throughs. Snapshots therefore match the raw | ||
| // structure we care about (╰▶ / ├▶ tree + labels + docs URL). | ||
|
|
||
| describe('NotInWorkflowContextError', () => { | ||
| it('frames the function name and docs link', () => { | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://workflow-sdk.dev/docs/api-reference/workflow/create-hook' | ||
| ); | ||
| expect(err.name).toBe('NotInWorkflowContextError'); | ||
| expect(err.message).toMatchInlineSnapshot(` | ||
| "\`createHook()\` can only be called inside a workflow function | ||
| ╰▶ docs: https://workflow-sdk.dev/docs/api-reference/workflow/create-hook" | ||
| `); | ||
| }); | ||
|
|
||
| it('does not expose functionName as an enumerable own property', () => { | ||
| // Regression: `readonly functionName` as a constructor param-property used | ||
| // to leak through util.inspect (Next.js error overlay, Node's default | ||
| // error formatter). Keep this invariant so the terminal output stays | ||
| // clean. | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://example.com/docs' | ||
| ); | ||
| expect(Object.keys(err)).not.toContain('functionName'); | ||
| expect((err as any).functionName).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('NotInStepContextError', () => { | ||
| it('uses "step function" phrasing', () => { | ||
| const err = new NotInStepContextError( | ||
| 'getStepMetadata()', | ||
| 'https://workflow-sdk.dev/docs/api-reference/workflow/get-step-metadata' | ||
| ); | ||
| expect(err.message).toContain('can only be called inside a step function'); | ||
| expect(err.message).toContain( | ||
| 'docs: https://workflow-sdk.dev/docs/api-reference/workflow/get-step-metadata' | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('NotInWorkflowOrStepContextError', () => { | ||
| it('uses "workflow or step function" phrasing', () => { | ||
| const err = new NotInWorkflowOrStepContextError( | ||
| 'getWorkflowMetadata()', | ||
| 'https://workflow-sdk.dev/docs/api-reference/workflow/get-workflow-metadata' | ||
| ); | ||
| expect(err.message).toContain( | ||
| 'can only be called inside a workflow or step function' | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('UnavailableInWorkflowContextError', () => { | ||
| afterEach(() => { | ||
| delete (globalThis as any)[WORKFLOW_CONTEXT_SYMBOL]; | ||
| }); | ||
|
|
||
| it('names the workflow when a context is active', () => { | ||
| (globalThis as any)[WORKFLOW_CONTEXT_SYMBOL] = { | ||
| workflowName: 'workflow//./src/workflows/example.ts//myWorkflow', | ||
| } as WorkflowMetadata; | ||
|
|
||
| const err = new UnavailableInWorkflowContextError( | ||
| 'resumeHook()', | ||
| 'https://workflow-sdk.dev/docs/api-reference/workflow-api/resume-hook' | ||
| ); | ||
| expect(err.message).toContain('cannot be called from a workflow context'); | ||
| expect(err.message).toContain( | ||
| 'workflow//./src/workflows/example.ts//myWorkflow' | ||
| ); | ||
| }); | ||
|
|
||
| it('falls back to a generic phrasing when no context is present', () => { | ||
| const err = new UnavailableInWorkflowContextError( | ||
| 'resumeHook()', | ||
| 'https://workflow-sdk.dev/docs/api-reference/workflow-api/resume-hook' | ||
| ); | ||
| expect(err.message).toContain('from a workflow context'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('plain .message / lazy pretty rendering', () => { | ||
| it('.message contains no ANSI escape bytes', () => { | ||
| // The user's structured logs, log drains, and CBOR event payloads all | ||
| // read `err.message` as a string. ANSI bytes leaking into them produced | ||
| // unreadable `\x1B[...m` noise in JSON. Keep `.message` plain. | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://example.com/docs' | ||
| ); | ||
| // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI check | ||
| expect(err.message).not.toMatch(/\x1B\[/); | ||
| }); | ||
|
|
||
| it('.stack contains no ANSI escape bytes', () => { | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://example.com/docs' | ||
| ); | ||
| // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI check | ||
| expect(err.stack ?? '').not.toMatch(/\x1B\[/); | ||
| }); | ||
|
|
||
| it('util.inspect(err) reveals the pretty framed form', () => { | ||
| // Node prints uncaught / logged errors via util.inspect. The pretty | ||
| // (framed) output belongs on the render path, not in stored state. | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://example.com/docs' | ||
| ); | ||
| const out = inspect(err); | ||
| expect(out).toContain('NotInWorkflowContextError:'); | ||
| expect(out).toContain('createHook()'); | ||
| expect(out).toContain('can only be called inside a workflow function'); | ||
| expect(out).toContain('╰▶'); | ||
| expect(out).toContain('docs:'); | ||
| }); | ||
|
|
||
| it('util.inspect(err) does not duplicate framed detail lines', () => { | ||
| // Regression: `.message` is multi-line (`title\n╰▶ docs: …`), so V8's | ||
| // `.stack` reads `Name: messageLine1\nmessageLine2\n at …`. Slicing | ||
| // only the first line of stack glued the framed-detail tail of the | ||
| // message onto the prepended pretty form and rendered every `╰▶ docs:` | ||
| // line twice. Now we slice past all message lines. | ||
| const out = inspect( | ||
| new NotInWorkflowContextError('createHook()', 'https://example.com/docs') | ||
| ); | ||
| // Multi-detail variants would also duplicate every detail; the docs | ||
| // line is the canonical case. | ||
| expect(out).not.toMatch(/╰▶ docs:.*\n.*╰▶ docs:/s); | ||
| // ╰▶ should appear exactly once for the single-detail error. | ||
| const occurrences = (out.match(/╰▶ docs:/g) ?? []).length; | ||
| expect(occurrences).toBe(1); | ||
| }); | ||
|
|
||
| it('err.toString() also returns the pretty framed form', () => { | ||
| const err = new NotInWorkflowContextError( | ||
| 'createHook()', | ||
| 'https://example.com/docs' | ||
| ); | ||
| expect(err.toString()).toContain('NotInWorkflowContextError:'); | ||
| expect(err.toString()).toContain('╰▶'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('FatalError.is() gate', () => { | ||
| // The step handler uses FatalError.is() to decide retry vs bubble-up. | ||
| // Context-violation errors can't succeed on retry — they signal the | ||
| // user called a workflow-only API from the wrong context — so burning | ||
| // three retry attempts just produces duplicated log output. | ||
| it.each([ | ||
| [ | ||
| 'NotInWorkflowContextError', | ||
| () => | ||
| new NotInWorkflowContextError('createHook()', 'https://example.com'), | ||
| ], | ||
| [ | ||
| 'NotInStepContextError', | ||
| () => | ||
| new NotInStepContextError('getStepMetadata()', 'https://example.com'), | ||
| ], | ||
| [ | ||
| 'NotInWorkflowOrStepContextError', | ||
| () => | ||
| new NotInWorkflowOrStepContextError( | ||
| 'getWorkflowMetadata()', | ||
| 'https://example.com' | ||
| ), | ||
| ], | ||
| [ | ||
| 'UnavailableInWorkflowContextError', | ||
| () => | ||
| new UnavailableInWorkflowContextError( | ||
| 'resumeHook()', | ||
| 'https://example.com' | ||
| ), | ||
| ], | ||
| ])('%s satisfies FatalError.is', (_name, make) => { | ||
| expect(FatalError.is(make())).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('throw helpers redirect the stack to the caller', () => { | ||
| // V8-only. Skip silently on engines without Error.captureStackTrace. | ||
| const hasCaptureStackTrace = | ||
| typeof (Error as unknown as { captureStackTrace?: unknown }) | ||
| .captureStackTrace === 'function'; | ||
|
|
||
| it.skipIf(!hasCaptureStackTrace)( | ||
| 'throwNotInWorkflowContext: top stack frame is the caller, not the framework function', | ||
| () => { | ||
| function frameworkGate() { | ||
| throwNotInWorkflowContext( | ||
| 'frameworkGate()', | ||
| 'https://example.com/docs', | ||
| frameworkGate | ||
| ); | ||
| } | ||
|
|
||
| function userCallSite() { | ||
| frameworkGate(); | ||
| } | ||
|
|
||
| try { | ||
| userCallSite(); | ||
| } catch (err) { | ||
| const stack = (err as Error).stack ?? ''; | ||
| // The first "at ..." frame should reference userCallSite, not | ||
| // frameworkGate or throwNotInWorkflowContext. | ||
| const firstFrame = stack | ||
| .split('\n') | ||
| .find((l) => l.trim().startsWith('at ')); | ||
| expect(firstFrame).toBeDefined(); | ||
| expect(firstFrame).toContain('userCallSite'); | ||
| expect(firstFrame).not.toContain('frameworkGate'); | ||
| expect(firstFrame).not.toContain('throwNotInWorkflowContext'); | ||
| return; | ||
| } | ||
| throw new Error('expected throwNotInWorkflowContext to throw'); | ||
| } | ||
| ); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.