From c48927b7c54d72e267423ab8eb785ccc80bfb0dc Mon Sep 17 00:00:00 2001 From: elpresidank Date: Thu, 4 Jun 2026 08:10:09 -0500 Subject: [PATCH] Use tagged errors in tests --- ts/EFFECT_NATIVE_REWRITE_AUDIT.md | 27 +++++++++++++++++++ .../src/__tests__/embeddings-service.test.ts | 25 ++++++++++++++--- .../__tests__/flow-processor-runtime.test.ts | 12 +++++++-- .../base/src/__tests__/nats-backend.test.ts | 22 +++++++++------ .../src/__tests__/runtime-services.test.ts | 10 +++++-- .../client/src/__tests__/messages.test.ts | 10 +++++-- ts/packages/client/src/models/messages.ts | 9 ++++--- .../src/__tests__/chunking-service.test.ts | 12 +++++++-- 8 files changed, 104 insertions(+), 23 deletions(-) diff --git a/ts/EFFECT_NATIVE_REWRITE_AUDIT.md b/ts/EFFECT_NATIVE_REWRITE_AUDIT.md index 93dd895c..d7debbba 100644 --- a/ts/EFFECT_NATIVE_REWRITE_AUDIT.md +++ b/ts/EFFECT_NATIVE_REWRITE_AUDIT.md @@ -2331,6 +2331,30 @@ Notes: - `cd ts && bun run lint` - `git diff --check` +### 2026-06-04: Tagged Test Error Cleanup Slice + +- Status: migrated and package-verified. +- Completed: + - Removed the remaining native `new Error` / `extends Error` matches from + tests and client wire models. + - Polling helpers in embeddings, flow processor, and chunking tests now fail + with `S.TaggedErrorClass` timeout values. + - Runtime service fake failures and the embeddings provider failure fixture + now use tagged test errors. + - The NATS SDK error mock preserves `code` and `jsError()` behavior through a + `TaggedErrorClass`, and the client model no longer exports a wire type + alias named `Error`. +- Verification: + - `cd ts/packages/base && bunx --bun vitest run src/__tests__/nats-backend.test.ts src/__tests__/runtime-services.test.ts src/__tests__/embeddings-service.test.ts src/__tests__/flow-processor-runtime.test.ts` + - `cd ts/packages/flow && bunx --bun vitest run src/__tests__/chunking-service.test.ts` + - `cd ts && bun run --cwd packages/client build && bun run --cwd packages/base build && bun run --cwd packages/flow build` + - `rg -n "new Error\\(|extends Error|export type Error|error\\?: Error|S\\.ErrorClass" ts/packages --glob '*.ts' --glob '*.tsx'` + - `cd ts && bun run check:tsgo` + - `cd ts && bun run build` + - `cd ts && bun run test` + - `cd ts && bun run lint` + - `git diff --check` + ## Subagent Findings To Preserve - MCP/workbench: @@ -2521,6 +2545,9 @@ Notes: standalone Librarian collection manager, prompt template cache, and workbench explain triples module cache. Local traversal sets and test fakes remain no-op boundaries. + - Fresh strict error-class sweep after the 2026-06-04 tagged test error + cleanup found no `new Error`, `extends Error`, `export type Error`, + `error?: Error`, or `S.ErrorClass` matches under `ts/packages`. ## Ranked Findings diff --git a/ts/packages/base/src/__tests__/embeddings-service.test.ts b/ts/packages/base/src/__tests__/embeddings-service.test.ts index d6989fdc..0f6ba8e1 100644 --- a/ts/packages/base/src/__tests__/embeddings-service.test.ts +++ b/ts/packages/base/src/__tests__/embeddings-service.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import { ConfigProvider, Effect, Fiber } from "effect"; +import * as S from "effect/Schema"; import { Embeddings, EmbeddingsService, @@ -18,6 +19,18 @@ import { type PubSubBackend, } from "../index.js"; +class WaitForTimeout extends S.TaggedErrorClass()( + "WaitForTimeout", + { label: S.String }, +) {} + +class TestProviderUnavailable extends S.TaggedErrorClass()( + "TestProviderUnavailable", + { message: S.String }, +) {} + +const isWaitForTimeout = S.is(WaitForTimeout); + function createMessage(value: T, properties: Record = {}): Message { return { value: () => value, @@ -36,14 +49,14 @@ const waitFor = (condition: () => boolean, label: string) => return; } if (Date.now() > deadline) { - reject(new Error(`Timed out waiting for ${label}`)); + reject(WaitForTimeout.make({ label })); return; } setTimeout(check, 5); }; check(); }), - catch: (error) => error, + catch: (error) => isWaitForTimeout(error) ? error : WaitForTimeout.make({ label }), }); class RecordingProducer implements BackendProducer { @@ -206,7 +219,13 @@ describe("EmbeddingsService", () => { const backend = new EmbeddingsBackend(); const embeddings = Embeddings.of({ embed: Effect.fn("FailingEmbeddings.embed")(() => - Effect.fail(embeddingsError("test.embed", new Error("provider unavailable"), "test")), + Effect.fail( + embeddingsError( + "test.embed", + TestProviderUnavailable.make({ message: "provider unavailable" }), + "test", + ), + ), ), }); diff --git a/ts/packages/base/src/__tests__/flow-processor-runtime.test.ts b/ts/packages/base/src/__tests__/flow-processor-runtime.test.ts index f11ad659..68f6a839 100644 --- a/ts/packages/base/src/__tests__/flow-processor-runtime.test.ts +++ b/ts/packages/base/src/__tests__/flow-processor-runtime.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import { ConfigProvider, Effect, Fiber } from "effect"; +import * as S from "effect/Schema"; import { FlowProcessor, MessagingRuntimeLive, @@ -17,6 +18,13 @@ import { type PubSubBackend, } from "../index.js"; +class WaitForTimeout extends S.TaggedErrorClass()( + "WaitForTimeout", + { label: S.String }, +) {} + +const isWaitForTimeout = S.is(WaitForTimeout); + function createMessage(value: T, properties: Record = {}): Message { return { value: () => value, @@ -35,14 +43,14 @@ const waitFor = (condition: () => boolean, label: string) => return; } if (Date.now() > deadline) { - reject(new Error(`Timed out waiting for ${label}`)); + reject(WaitForTimeout.make({ label })); return; } setTimeout(check, 5); }; check(); }), - catch: (error) => error, + catch: (error) => isWaitForTimeout(error) ? error : WaitForTimeout.make({ label }), }); class RecordingProducer implements BackendProducer { diff --git a/ts/packages/base/src/__tests__/nats-backend.test.ts b/ts/packages/base/src/__tests__/nats-backend.test.ts index 75c0a007..e6bf942d 100644 --- a/ts/packages/base/src/__tests__/nats-backend.test.ts +++ b/ts/packages/base/src/__tests__/nats-backend.test.ts @@ -2,18 +2,24 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { makeNatsBackend } from "../backend/nats.js"; const natsMock = vi.hoisted(() => { + const S = require("effect/Schema"); const encoder = new TextEncoder(); const decoder = new TextDecoder(); - class MockNatsError extends Error { - readonly code: string; - private readonly apiCode: number | undefined; - + class MockNatsError extends S.TaggedErrorClass()( + "MockNatsError", + { + apiCode: S.optional(S.Number), + code: S.String, + message: S.String, + }, + ) { constructor(code: string, apiCode?: number) { - super(code); - this.name = "NatsError"; - this.code = code; - this.apiCode = apiCode; + super({ + apiCode, + code, + message: code, + }); } jsError() { diff --git a/ts/packages/base/src/__tests__/runtime-services.test.ts b/ts/packages/base/src/__tests__/runtime-services.test.ts index 98207779..3983a88a 100644 --- a/ts/packages/base/src/__tests__/runtime-services.test.ts +++ b/ts/packages/base/src/__tests__/runtime-services.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect } from "effect"; +import * as S from "effect/Schema"; import { PubSub, makeAsyncProcessor, @@ -13,6 +14,11 @@ import { type PubSubBackend, } from "../index.js"; +class RuntimeServicesTestError extends S.TaggedErrorClass()( + "RuntimeServicesTestError", + { message: S.String }, +) {} + class FakeProducer implements BackendProducer { readonly sent: Array<{ readonly message: T; readonly properties?: Record }> = []; closeCount = 0; @@ -75,7 +81,7 @@ class FakePubSubBackend implements PubSubBackend { class FailingProducerBackend extends FakePubSubBackend { override async createProducer(): Promise> { - throw new Error("producer unavailable"); + throw RuntimeServicesTestError.make({ message: "producer unavailable" }); } } @@ -99,7 +105,7 @@ const makeRecordingProcessor = ( const makeFailingProcessor = (config: ProcessorConfig) => makeAsyncProcessor(config, { run: async () => { - throw new Error("processor failed"); + throw RuntimeServicesTestError.make({ message: "processor failed" }); }, }); diff --git a/ts/packages/client/src/__tests__/messages.test.ts b/ts/packages/client/src/__tests__/messages.test.ts index 65d96c9e..33939b00 100644 --- a/ts/packages/client/src/__tests__/messages.test.ts +++ b/ts/packages/client/src/__tests__/messages.test.ts @@ -324,7 +324,10 @@ describe("Message Types", () => { describe("LibraryResponse", () => { it("should have correct structure", () => { const response: LibraryResponse = { - error: new Error(), + error: { + message: "Library unavailable", + type: "library-error", + }, "document-metadatas": [ { id: "doc-1", @@ -334,7 +337,10 @@ describe("Message Types", () => { ], }; - expect(response.error).toBeInstanceOf(Error); + expect(response.error).toMatchObject({ + message: "Library unavailable", + type: "library-error", + }); expect(response["document-metadatas"]).toHaveLength(1); expect(response["document-metadatas"]![0].id).toBe("doc-1"); }); diff --git a/ts/packages/client/src/models/messages.ts b/ts/packages/client/src/models/messages.ts index f7f84c21..892724c1 100644 --- a/ts/packages/client/src/models/messages.ts +++ b/ts/packages/client/src/models/messages.ts @@ -2,13 +2,14 @@ import type { Term, Triple } from "./Triple.js"; export type Request = object; export type Response = object; -export type Error = object | string; export interface ResponseError { type?: string; message: string; } +export type WireError = object | string; + export interface RequestMessage { id: string; service: string; @@ -315,7 +316,7 @@ export interface LibraryRequest { } export interface LibraryResponse { - error?: Error; + error?: WireError; "document-metadata"?: DocumentMetadata; documentMetadata?: DocumentMetadata; content?: string; @@ -340,7 +341,7 @@ export interface KnowledgeRequest { } export interface KnowledgeResponse { - error?: Error; + error?: WireError; ids?: string[]; eos?: boolean; triples?: Triple[]; @@ -371,7 +372,7 @@ export interface FlowResponse { | { message?: string; } - | Error; + | WireError; } export interface PromptRequest { diff --git a/ts/packages/flow/src/__tests__/chunking-service.test.ts b/ts/packages/flow/src/__tests__/chunking-service.test.ts index aed6121e..f83a1552 100644 --- a/ts/packages/flow/src/__tests__/chunking-service.test.ts +++ b/ts/packages/flow/src/__tests__/chunking-service.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import { ConfigProvider, Effect, Fiber } from "effect"; import * as EffectChunk from "effect/Chunk"; +import * as S from "effect/Schema"; import { MessagingRuntimeLive, PubSub, @@ -18,6 +19,13 @@ import { import { ChunkingService } from "../chunking/service.js"; import { recursiveSplit } from "../chunking/recursive-splitter.js"; +class WaitForTimeout extends S.TaggedErrorClass()( + "WaitForTimeout", + { label: S.String }, +) {} + +const isWaitForTimeout = S.is(WaitForTimeout); + function createMessage(value: T, properties: Record = {}): Message { return { value: () => value, @@ -36,14 +44,14 @@ const waitFor = (condition: () => boolean, label: string) => return; } if (Date.now() > deadline) { - reject(new Error(`Timed out waiting for ${label}`)); + reject(WaitForTimeout.make({ label })); return; } setTimeout(check, 5); }; check(); }), - catch: (error) => error, + catch: (error) => isWaitForTimeout(error) ? error : WaitForTimeout.make({ label }), }); class RecordingProducer implements BackendProducer {