Standardize pipeline error shape across stages and web workflow#24
Standardize pipeline error shape across stages and web workflow#24Boykosoft wants to merge 1 commit intoOpen-scribe:mainfrom
Conversation
| @@ -0,0 +1,22 @@ | |||
| import assert from "node:assert/strict" | |||
| import test from "node:test" | |||
| import { transcriptionSessionStore } from "../session-store.js" | |||
There was a problem hiding this comment.
Right now this import instantiates the global TranscriptionSessionStore singleton, which starts a perpetual setInterval in its constructor. With this new test now included in the test suite, the pnpm test:run hangs after tests pass due to open handles. Could you please add interval lifecycle safety (e.g., unref(), explicit dispose/teardown, or a non-singleton test instance) so the test runner can exit cleanly.
| let message = `Final upload failed (${response.status})` | ||
| try { | ||
| const body = (await response.json()) as { error?: { message?: string } } | ||
| const body = (await response.json()) as { error?: PipelineError } |
There was a problem hiding this comment.
we parse a structured PipelineError here, but later the flow throws a plain Error(message). In the outer catch, toPipelineError(..., { recoverable: true }) can reclassify unrecoverable failures as recoverable and drop server-provided code/details. Please preserve and rethrow the parsed PipelineError (or PipelineStageError) rather than converting it to error.
| if (body?.error?.message) { | ||
| message = body.error.message | ||
| } | ||
| if (body?.error) { |
There was a problem hiding this comment.
Could you add a test for final-upload failure propagation that verifies server-provided { code, message, recoverable, details } reaches workflow error state unchanged (no re-normalization to generic api_error/recoverable=true).
| ) | ||
| } | ||
|
|
||
| export function toPipelineError( |
There was a problem hiding this comment.
This is a great extraction. This honestly makes downstream stages much cleaner and reduces ad-hoc error mapping drift.
| onRetry?: () => void | ||
| } | ||
|
|
||
| export function WorkflowErrorDisplay({ error, onRetry }: WorkflowErrorDisplayProps) { |
There was a problem hiding this comment.
This helps pulling this into a dedicated component keeps page.tsx from growing
sammargolis
left a comment
sammargolis
left a comment
There was a problem hiding this comment.
This is a really great direction on standardizing pipeline errors and surfacing recoverability in the UI/API. I found two issues that should be addressed before merge. Right now (1) the test runner hang from session-store interval lifecycle, and (2) the final upload error rethrow dropping structured metadata/recoverability.
2 participants
Summary
Introduce a shared
PipelineErrorshape and apply it consistently across all four pipeline stages and the web workflow UI.Problem
Each pipeline stage (audio-ingest, transcribe, assemble, note-core) handled errors differently — some threw plain
Error, others returned ad-hoc objects, and the web app had no unified way to display failures or offer retry actions.Solution
Shared error contract
packages/pipeline/shared/src/error.tswith:PipelineErrorinterface (code,message,recoverable,details?)PipelineStageErrorclass extendingErrorcreatePipelineError,isPipelineError,toPipelineError,toPipelineStageErrorUpdated stages
toAudioIngestErrorPipelineStageErrorsession-store.emitErroremits standardized error eventscreateClinicalNoteTextthrowsPipelineStageErroron API failuresWeb workflow
WorkflowErrorDisplaycomponent renders error message + conditional Retry button whenrecoverable: trueapps/web/src/app/page.tsxTests
One test per stage confirming the standardized error shape:
audio-processing.test.ts— capture error normalizationtranscribe.test.ts— WAV validation error shapesession-store.test.ts— assembly error emissionnote-generator.test.ts— note generation failure shapeCloses #12