From 00cdf418996227f49ec2b45fde180a5230cb99e2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 26 Feb 2026 18:32:39 -0800 Subject: [PATCH] Harden server async error handling with typed Effect errors - Replace raw `Effect.promise`/generic errors with `Effect.tryPromise` and tagged schema errors - Improve orchestration harness/test reliability with explicit timeout/runtime error types - Tighten related tests and error assertions to match typed error flows --- .../OrchestrationEngineHarness.integration.ts | 79 ++++++++++++------- .../orchestrationEngine.integration.test.ts | 23 +++++- apps/server/src/git/Layers/GitManager.test.ts | 35 ++++---- apps/server/src/keybindings.test.ts | 8 +- apps/server/src/open.ts | 5 +- apps/server/src/terminal/Layers/NodePTY.ts | 19 ++++- apps/server/src/wsServer.ts | 8 +- 7 files changed, 121 insertions(+), 56 deletions(-) diff --git a/apps/server/integration/OrchestrationEngineHarness.integration.ts b/apps/server/integration/OrchestrationEngineHarness.integration.ts index b783191f338916669cfbe4633cec6b21fb20f9fa..696e24f036b60f0e032b684d6daab0e70a11b97e 100644 --- a/apps/server/integration/OrchestrationEngineHarness.integration.ts +++ b/apps/server/integration/OrchestrationEngineHarness.integration.ts @@ -9,7 +9,7 @@ import { type OrchestrationEvent, type OrchestrationThread, } from "@t3tools/contracts"; -import { Effect, Exit, Layer, ManagedRuntime, Option, Schedule, Scope, Stream } from "effect"; +import { Effect, Exit, Layer, ManagedRuntime, Option, Schedule, Schema, Scope, Stream } from "effect"; import { CheckpointStoreLive } from "../src/checkpointing/Layers/CheckpointStore.ts"; import { CheckpointStore } from "../src/checkpointing/Services/CheckpointStore.ts"; @@ -75,20 +75,24 @@ export function gitShowFileAtRef(cwd: string, ref: string, filePath: string): st return runGit(cwd, ["show", `${ref}:${filePath}`]); } -function waitFor( - read: Effect.Effect, +class WaitForTimeoutError extends Schema.TaggedErrorClass()("WaitForTimeoutError", { + description: Schema.String, +}) {} + +function waitFor( + read: Effect.Effect, predicate: (value: A) => boolean, description: string, timeoutMs?: number, ): Effect.Effect; -function waitFor( - read: Effect.Effect, +function waitFor( + read: Effect.Effect, predicate: (value: A) => value is B, description: string, timeoutMs?: number, ): Effect.Effect; -function waitFor( - read: Effect.Effect, +function waitFor( + read: Effect.Effect, predicate: (value: A) => boolean, description: string, timeoutMs = 3000, @@ -106,12 +110,26 @@ function waitFor( while: (error) => error === RETRY_SIGNAL, }), Effect.mapError((error) => - error === RETRY_SIGNAL ? new Error(`Timed out waiting for ${description}.`) : error, + error === RETRY_SIGNAL ? new WaitForTimeoutError({ description }) : error, ), Effect.orDie, ); } +class OrchestrationHarnessRuntimeError extends Schema.TaggedErrorClass()( + "OrchestrationHarnessRuntimeError", + { + operation: Schema.String, + cause: Schema.optional(Schema.Defect), + }, +) {} + +const tryRuntimePromise = (operation: string, run: () => Promise) => + Effect.tryPromise({ + try: run, + catch: (cause) => new OrchestrationHarnessRuntimeError({ operation, cause }), + }); + export interface OrchestrationIntegrationHarness { readonly rootDir: string; readonly workspaceDir: string; @@ -153,7 +171,10 @@ export interface OrchestrationIntegrationHarness { export const makeOrchestrationIntegrationHarness = Effect.gen(function* () { const sleep = (ms: number) => - Effect.promise(() => new Promise((resolve) => setTimeout(resolve, ms))); + Effect.tryPromise({ + try: () => new Promise((resolve) => setTimeout(resolve, ms)), + catch: (cause) => new OrchestrationHarnessRuntimeError({ operation: "sleep", cause }), + }).pipe(Effect.orDie); const adapterHarness = yield* makeTestProviderAdapterHarness; const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-orchestration-integration-")); @@ -215,30 +236,34 @@ export const makeOrchestrationIntegrationHarness = Effect.gen(function* () { ); const runtime = ManagedRuntime.make(layer); - const engine = yield* Effect.promise(() => + const engine = yield* tryRuntimePromise("load OrchestrationEngine service", () => runtime.runPromise(Effect.service(OrchestrationEngineService)), - ); - const reactor = yield* Effect.promise(() => + ).pipe(Effect.orDie); + const reactor = yield* tryRuntimePromise("load OrchestrationReactor service", () => runtime.runPromise(Effect.service(OrchestrationReactor)), - ); - const snapshotQuery = yield* Effect.promise(() => + ).pipe(Effect.orDie); + const snapshotQuery = yield* tryRuntimePromise("load ProjectionSnapshotQuery service", () => runtime.runPromise(Effect.service(ProjectionSnapshotQuery)), - ); - const providerService = yield* Effect.promise(() => + ).pipe(Effect.orDie); + const providerService = yield* tryRuntimePromise("load ProviderService service", () => runtime.runPromise(Effect.service(ProviderService)), - ); - const checkpointStore = yield* Effect.promise(() => + ).pipe(Effect.orDie); + const checkpointStore = yield* tryRuntimePromise("load CheckpointStore service", () => runtime.runPromise(Effect.service(CheckpointStore)), - ); - const checkpointRepository = yield* Effect.promise(() => - runtime.runPromise(Effect.service(ProjectionCheckpointRepository)), - ); - const pendingApprovalRepository = yield* Effect.promise(() => - runtime.runPromise(Effect.service(ProjectionPendingApprovalRepository)), - ); + ).pipe(Effect.orDie); + const checkpointRepository = yield* tryRuntimePromise( + "load ProjectionCheckpointRepository service", + () => runtime.runPromise(Effect.service(ProjectionCheckpointRepository)), + ).pipe(Effect.orDie); + const pendingApprovalRepository = yield* tryRuntimePromise( + "load ProjectionPendingApprovalRepository service", + () => runtime.runPromise(Effect.service(ProjectionPendingApprovalRepository)), + ).pipe(Effect.orDie); - const scope = yield* Effect.promise(() => Effect.runPromise(Scope.make("sequential"))); - yield* Effect.promise(() => runtime.runPromise(reactor.start.pipe(Scope.provide(scope)))); + const scope = yield* Scope.make("sequential"); + yield* tryRuntimePromise("start OrchestrationReactor", () => + runtime.runPromise(reactor.start.pipe(Scope.provide(scope))), + ).pipe(Effect.orDie); yield* sleep(10); const waitForThread: OrchestrationIntegrationHarness["waitForThread"] = ( diff --git a/apps/server/integration/orchestrationEngine.integration.test.ts b/apps/server/integration/orchestrationEngine.integration.test.ts index 010be960a2fd1464f4be1626c1258f83eddaf356..91afa283ee44bdc2f36a91d29ea1467361994634 100644 --- a/apps/server/integration/orchestrationEngine.integration.test.ts +++ b/apps/server/integration/orchestrationEngine.integration.test.ts @@ -13,7 +13,7 @@ import { ThreadId, } from "@t3tools/contracts"; import { assert, it } from "@effect/vitest"; -import { Effect, Exit, Option } from "effect"; +import { Effect, Exit, Option, Schema } from "effect"; import type { TestTurnResponse } from "./TestProviderAdapter.integration.ts"; import { @@ -45,8 +45,25 @@ function nowIso() { return new Date().toISOString(); } +class IntegrationSleepError extends Schema.TaggedErrorClass()( + "IntegrationSleepError", + { + cause: Schema.optional(Schema.Defect), + }, +) {} + +class IntegrationWaitTimeoutError extends Schema.TaggedErrorClass()( + "IntegrationWaitTimeoutError", + { + description: Schema.String, + }, +) {} + const sleep = (ms: number) => - Effect.promise(() => new Promise((resolve) => setTimeout(resolve, ms))); + Effect.tryPromise({ + try: () => new Promise((resolve) => setTimeout(resolve, ms)), + catch: (cause) => new IntegrationSleepError({ cause }), + }).pipe(Effect.orDie); function waitForSync( read: () => A, @@ -63,7 +80,7 @@ function waitForSync( return value; } if (Date.now() >= deadline) { - return yield* Effect.die(new Error(`Timed out waiting for ${description}.`)); + return yield* Effect.die(new IntegrationWaitTimeoutError({ description })); } yield* sleep(10); } diff --git a/apps/server/src/git/Layers/GitManager.test.ts b/apps/server/src/git/Layers/GitManager.test.ts index 673de05b7242a60908a40297dfecaddfa60c6a02..3417b58c2a274e86e3201966db177c49d7cc2b41 100644 --- a/apps/server/src/git/Layers/GitManager.test.ts +++ b/apps/server/src/git/Layers/GitManager.test.ts @@ -3,10 +3,10 @@ import path from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it } from "@effect/vitest"; -import { Effect, FileSystem, Layer, PlatformError, Scope } from "effect"; +import { Effect, FileSystem, Layer, PlatformError, Schema, Scope } from "effect"; import { expect } from "vitest"; -import { GitHubCliError, TextGenerationError } from "../Errors.ts"; +import { GitCommandError, GitHubCliError, TextGenerationError } from "../Errors.ts"; import { type GitManagerShape } from "../Services/GitManager.ts"; import { type GitHubCliShape, @@ -19,11 +19,16 @@ import { GitService } from "../Services/GitService.ts"; import { GitCoreLive } from "./GitCore.ts"; import { makeGitManager } from "./GitManager.ts"; +class GitFixtureError extends Schema.TaggedErrorClass()("GitFixtureError", { + message: Schema.String, + cause: Schema.optional(Schema.Defect), +}) {} + interface FakeGhScenario { prListSequence?: string[]; createdPrUrl?: string; defaultBranch?: string; - failWith?: Error; + failWith?: GitFixtureError; } interface FakeGitTextGeneration { @@ -32,7 +37,7 @@ interface FakeGitTextGeneration { branch: string | null; stagedSummary: string; stagedPatch: string; - }) => Effect.Effect<{ subject: string; body: string }, unknown>; + }) => Effect.Effect<{ subject: string; body: string }, GitFixtureError>; generatePrContent: (input: { cwd: string; baseBranch: string; @@ -40,7 +45,7 @@ interface FakeGitTextGeneration { commitSummary: string; diffSummary: string; diffPatch: string; - }) => Effect.Effect<{ title: string; body: string }, unknown>; + }) => Effect.Effect<{ title: string; body: string }, GitFixtureError>; } function makeTempDir( @@ -58,7 +63,7 @@ function runGit( allowNonZeroExit = false, ): Effect.Effect< { readonly code: number; readonly stdout: string; readonly stderr: string }, - unknown, + GitCommandError, GitService > { return Effect.gen(function* () { @@ -74,7 +79,7 @@ function runGit( function initRepo( cwd: string, -): Effect.Effect { +): Effect.Effect { return Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; yield* runGit(cwd, ["init", "--initial-branch=main"]); @@ -88,7 +93,7 @@ function initRepo( function createBareRemote(): Effect.Effect< string, - unknown, + PlatformError.PlatformError | GitCommandError, FileSystem.FileSystem | Scope.Scope | GitService > { return Effect.gen(function* () { @@ -139,7 +144,7 @@ function createTextGeneration(overrides: Partial = {}): T }; } -const normalizeGhError = (error: Error): GitHubCliError => { +const normalizeGhError = (error: GitFixtureError): GitHubCliError => { if (error.message.includes("Command not found: gh")) { return new GitHubCliError({ operation: "execute", @@ -373,7 +378,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager } = yield* makeManager({ ghScenario: { - failWith: new Error("Command not found: gh"), + failWith: new GitFixtureError({ message: "Command not found: gh" }), }, }); @@ -600,7 +605,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { action: "commit_push", }).pipe( Effect.flip, - Effect.map((error) => (error instanceof Error ? error.message : String(error))), + Effect.map((error) => error.message), ); expect(errorMessage).toContain("detached HEAD"); }), @@ -617,7 +622,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager } = yield* makeManager({ ghScenario: { - failWith: new Error("Command not found: gh"), + failWith: new GitFixtureError({ message: "Command not found: gh" }), }, }); @@ -626,7 +631,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { action: "commit_push_pr", }).pipe( Effect.flip, - Effect.map((error) => (error instanceof Error ? error.message : String(error))), + Effect.map((error) => error.message), ); expect(errorMessage).toContain("GitHub CLI (`gh`) is required"); }), @@ -643,7 +648,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager } = yield* makeManager({ ghScenario: { - failWith: new Error("authentication failed"), + failWith: new GitFixtureError({ message: "authentication failed" }), }, }); @@ -652,7 +657,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { action: "commit_push_pr", }).pipe( Effect.flip, - Effect.map((error) => (error instanceof Error ? error.message : String(error))), + Effect.map((error) => error.message), ); expect(errorMessage).toContain("gh auth login"); }), diff --git a/apps/server/src/keybindings.test.ts b/apps/server/src/keybindings.test.ts index 2eb8dac0acfdca8048aea79b256e481d06ac2ae9..4a10d5816b4f3a08ab77d2bd609021f2faf40f18 100644 --- a/apps/server/src/keybindings.test.ts +++ b/apps/server/src/keybindings.test.ts @@ -211,13 +211,7 @@ it.layer(NodeServices.layer)("keybindings", (it) => { }); assert.isTrue(configState.keybindings.some((entry) => entry.command === "terminal.toggle")); - assert.isFalse( - configState.keybindings.some( - (entry) => - // @ts-expect-error - invalid command - entry.command === "invalid.command", - ), - ); + assert.isFalse(configState.keybindings.some((entry) => String(entry.command) === "invalid.command")); assert.deepEqual(configState.issues, [ { kind: "keybindings.invalid-entry", diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts index a3ce1e87a96901b74ea33a282df4c503ba1b22cb..97c822827d600475bc865b67a3b087cba8b7109a 100644 --- a/apps/server/src/open.ts +++ b/apps/server/src/open.ts @@ -117,7 +117,10 @@ export const launchDetached = (launch: EditorLaunch) => }); const make = Effect.gen(function* () { - const open = yield* Effect.promise(() => import("open")); + const open = yield* Effect.tryPromise({ + try: () => import("open"), + catch: (cause) => new OpenError({ message: "failed to load browser opener", cause }), + }); return { openBrowser: (target) => diff --git a/apps/server/src/terminal/Layers/NodePTY.ts b/apps/server/src/terminal/Layers/NodePTY.ts index 60a7ab6220898d5716edde9167e96a4632935544..a45d46f23fb0d5bd74f61be229a3355f7f115d24 100644 --- a/apps/server/src/terminal/Layers/NodePTY.ts +++ b/apps/server/src/terminal/Layers/NodePTY.ts @@ -1,10 +1,18 @@ import { createRequire } from "node:module"; -import { Effect, FileSystem, Layer, Path } from "effect"; +import { Effect, FileSystem, Layer, Path, Schema } from "effect"; import { PtyAdapter, PtyAdapterShape, PtyExitEvent, PtyProcess } from "../Services/PTY"; let didEnsureSpawnHelperExecutable = false; +export class NodePtyAdapterError extends Schema.TaggedErrorClass()( + "NodePtyAdapterError", + { + message: Schema.String, + cause: Schema.optional(Schema.Defect), + }, +) {} + const resolveNodePtySpawnHelperPath = Effect.gen(function* () { const requireForNodePty = createRequire(import.meta.url); const path = yield* Path.Path; @@ -90,7 +98,14 @@ export const NodePtyAdapterLive = Layer.effect( const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; - const nodePty = yield* Effect.promise(() => import("node-pty")); + const nodePty = yield* Effect.tryPromise({ + try: () => import("node-pty"), + catch: (cause) => + new NodePtyAdapterError({ + message: "failed to load node-pty module", + cause, + }), + }); const ensureNodePtySpawnHelperExecutableCached = yield* Effect.cached( ensureNodePtySpawnHelperExecutable().pipe( diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index 0a17b0dc4266ac40fbef3ffd0788f45db1f0890c..85986f35e73892e99abef2d7e53b38bb1dc7bc53 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -389,7 +389,13 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< case WS_METHODS.projectsSearchEntries: { const body = stripRequestTag(request.body); - return yield* Effect.promise(() => searchWorkspaceEntries(body)); + return yield* Effect.tryPromise({ + try: () => searchWorkspaceEntries(body), + catch: (cause) => + new RouteRequestError({ + message: `Failed to search workspace entries: ${String(cause)}`, + }), + }); } case WS_METHODS.shellOpenInEditor: { -- 2.54.0