History

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

Julius Marminge committed 8 months ago

Commit 00cdf411 parent 1b982137 files · +121 −56Browse files

Unified · Split

7 files
  1. apps/server/integration/OrchestrationEngineHarness.integration.ts +52 −27
  2. apps/server/integration/orchestrationEngine.integration.test.ts +20 −3
  3. apps/server/src/git/Layers/GitManager.test.ts +20 −15
  4. apps/server/src/keybindings.test.ts +1 −7
  5. apps/server/src/open.ts +4 −1
  6. apps/server/src/terminal/Layers/NodePTY.ts +17 −2
  7. apps/server/src/wsServer.ts +7 −1
apps/server/integration/OrchestrationEngineHarness.integration.ts+52−27
@@ -9,7 +9,7 @@ import { · 8 unmodified lines
9 type OrchestrationEvent,9 type OrchestrationEvent,
10 type OrchestrationThread,10 type OrchestrationThread,
11} from "@t3tools/contracts";11} from "@t3tools/contracts";
12import { Effect, Exit, Layer, ManagedRuntime, Option, Schedule, Scope, Stream } from "effect";12import { Effect, Exit, Layer, ManagedRuntime, Option, Schedule, Schema, Scope, Stream } from "effect";
13 13
14import { CheckpointStoreLive } from "../src/checkpointing/Layers/CheckpointStore.ts";14import { CheckpointStoreLive } from "../src/checkpointing/Layers/CheckpointStore.ts";
15import { CheckpointStore } from "../src/checkpointing/Services/CheckpointStore.ts";15import { CheckpointStore } from "../src/checkpointing/Services/CheckpointStore.ts";
@@ -75,20 +75,24 @@ export function gitShowFileAtRef(cwd: string, ref: string, filePath: string): st · 59 unmodified lines
75 return runGit(cwd, ["show", `${ref}:${filePath}`]);75 return runGit(cwd, ["show", `${ref}:${filePath}`]);
76}76}
77 77
78function waitFor<A>(78class WaitForTimeoutError extends Schema.TaggedErrorClass<WaitForTimeoutError>()("WaitForTimeoutError", {
79 read: Effect.Effect<A, unknown>,79 description: Schema.String,
80}) {}
81
82function waitFor<A, E>(
83 read: Effect.Effect<A, E>,
80 predicate: (value: A) => boolean,84 predicate: (value: A) => boolean,
81 description: string,85 description: string,
82 timeoutMs?: number,86 timeoutMs?: number,
83): Effect.Effect<A, never>;87): Effect.Effect<A, never>;
84function waitFor<A, B extends A>(88function waitFor<A, B extends A, E>(
85 read: Effect.Effect<A, unknown>,89 read: Effect.Effect<A, E>,
86 predicate: (value: A) => value is B,90 predicate: (value: A) => value is B,
87 description: string,91 description: string,
88 timeoutMs?: number,92 timeoutMs?: number,
89): Effect.Effect<B, never>;93): Effect.Effect<B, never>;
90function waitFor<A>(94function waitFor<A, E>(
91 read: Effect.Effect<A, unknown>,95 read: Effect.Effect<A, E>,
92 predicate: (value: A) => boolean,96 predicate: (value: A) => boolean,
93 description: string,97 description: string,
94 timeoutMs = 3000,98 timeoutMs = 3000,
@@ -106,12 +110,26 @@ function waitFor<A>( · 11 unmodified lines
106 while: (error) => error === RETRY_SIGNAL,110 while: (error) => error === RETRY_SIGNAL,
107 }),111 }),
108 Effect.mapError((error) =>112 Effect.mapError((error) =>
109 error === RETRY_SIGNAL ? new Error(`Timed out waiting for ${description}.`) : error,113 error === RETRY_SIGNAL ? new WaitForTimeoutError({ description }) : error,
110 ),114 ),
111 Effect.orDie,115 Effect.orDie,
112 );116 );
113}117}
114 118
119class OrchestrationHarnessRuntimeError extends Schema.TaggedErrorClass<OrchestrationHarnessRuntimeError>()(
120 "OrchestrationHarnessRuntimeError",
121 {
122 operation: Schema.String,
123 cause: Schema.optional(Schema.Defect),
124 },
125) {}
126
127const tryRuntimePromise = <A>(operation: string, run: () => Promise<A>) =>
128 Effect.tryPromise({
129 try: run,
130 catch: (cause) => new OrchestrationHarnessRuntimeError({ operation, cause }),
131 });
132
115export interface OrchestrationIntegrationHarness {133export interface OrchestrationIntegrationHarness {
116 readonly rootDir: string;134 readonly rootDir: string;
117 readonly workspaceDir: string;135 readonly workspaceDir: string;
@@ -153,7 +171,10 @@ export interface OrchestrationIntegrationHarness { · 35 unmodified lines
153 171
154export const makeOrchestrationIntegrationHarness = Effect.gen(function* () {172export const makeOrchestrationIntegrationHarness = Effect.gen(function* () {
155 const sleep = (ms: number) =>173 const sleep = (ms: number) =>
156 Effect.promise(() => new Promise<void>((resolve) => setTimeout(resolve, ms)));174 Effect.tryPromise({
175 try: () => new Promise<void>((resolve) => setTimeout(resolve, ms)),
176 catch: (cause) => new OrchestrationHarnessRuntimeError({ operation: "sleep", cause }),
177 }).pipe(Effect.orDie);
157 const adapterHarness = yield* makeTestProviderAdapterHarness;178 const adapterHarness = yield* makeTestProviderAdapterHarness;
158 179
159 const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-orchestration-integration-"));180 const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-orchestration-integration-"));
@@ -215,30 +236,34 @@ export const makeOrchestrationIntegrationHarness = Effect.gen(function* () { · 55 unmodified lines
215 );236 );
216 237
217 const runtime = ManagedRuntime.make(layer);238 const runtime = ManagedRuntime.make(layer);
218 const engine = yield* Effect.promise(() =>239 const engine = yield* tryRuntimePromise("load OrchestrationEngine service", () =>
219 runtime.runPromise(Effect.service(OrchestrationEngineService)),240 runtime.runPromise(Effect.service(OrchestrationEngineService)),
220 );241 ).pipe(Effect.orDie);
221 const reactor = yield* Effect.promise(() =>242 const reactor = yield* tryRuntimePromise("load OrchestrationReactor service", () =>
222 runtime.runPromise(Effect.service(OrchestrationReactor)),243 runtime.runPromise(Effect.service(OrchestrationReactor)),
223 );244 ).pipe(Effect.orDie);
224 const snapshotQuery = yield* Effect.promise(() =>245 const snapshotQuery = yield* tryRuntimePromise("load ProjectionSnapshotQuery service", () =>
225 runtime.runPromise(Effect.service(ProjectionSnapshotQuery)),246 runtime.runPromise(Effect.service(ProjectionSnapshotQuery)),
226 );247 ).pipe(Effect.orDie);
227 const providerService = yield* Effect.promise(() =>248 const providerService = yield* tryRuntimePromise("load ProviderService service", () =>
228 runtime.runPromise(Effect.service(ProviderService)),249 runtime.runPromise(Effect.service(ProviderService)),
229 );250 ).pipe(Effect.orDie);
230 const checkpointStore = yield* Effect.promise(() =>251 const checkpointStore = yield* tryRuntimePromise("load CheckpointStore service", () =>
231 runtime.runPromise(Effect.service(CheckpointStore)),252 runtime.runPromise(Effect.service(CheckpointStore)),
232 );253 ).pipe(Effect.orDie);
233 const checkpointRepository = yield* Effect.promise(() =>254 const checkpointRepository = yield* tryRuntimePromise(
234 runtime.runPromise(Effect.service(ProjectionCheckpointRepository)),255 "load ProjectionCheckpointRepository service",
235 );256 () => runtime.runPromise(Effect.service(ProjectionCheckpointRepository)),
236 const pendingApprovalRepository = yield* Effect.promise(() =>257 ).pipe(Effect.orDie);
237 runtime.runPromise(Effect.service(ProjectionPendingApprovalRepository)),258 const pendingApprovalRepository = yield* tryRuntimePromise(
238 );259 "load ProjectionPendingApprovalRepository service",
260 () => runtime.runPromise(Effect.service(ProjectionPendingApprovalRepository)),
261 ).pipe(Effect.orDie);
239 262
240 const scope = yield* Effect.promise(() => Effect.runPromise(Scope.make("sequential")));263 const scope = yield* Scope.make("sequential");
241 yield* Effect.promise(() => runtime.runPromise(reactor.start.pipe(Scope.provide(scope))));264 yield* tryRuntimePromise("start OrchestrationReactor", () =>
265 runtime.runPromise(reactor.start.pipe(Scope.provide(scope))),
266 ).pipe(Effect.orDie);
242 yield* sleep(10);267 yield* sleep(10);
243 268
244 const waitForThread: OrchestrationIntegrationHarness["waitForThread"] = (269 const waitForThread: OrchestrationIntegrationHarness["waitForThread"] = (
apps/server/integration/orchestrationEngine.integration.test.ts+20−3
@@ -13,7 +13,7 @@ import { · 12 unmodified lines
13 ThreadId,13 ThreadId,
14} from "@t3tools/contracts";14} from "@t3tools/contracts";
15import { assert, it } from "@effect/vitest";15import { assert, it } from "@effect/vitest";
16import { Effect, Exit, Option } from "effect";16import { Effect, Exit, Option, Schema } from "effect";
17 17
18import type { TestTurnResponse } from "./TestProviderAdapter.integration.ts";18import type { TestTurnResponse } from "./TestProviderAdapter.integration.ts";
19import {19import {
@@ -45,8 +45,25 @@ function nowIso() { · 25 unmodified lines
45 return new Date().toISOString();45 return new Date().toISOString();
46}46}
47 47
48class IntegrationSleepError extends Schema.TaggedErrorClass<IntegrationSleepError>()(
49 "IntegrationSleepError",
50 {
51 cause: Schema.optional(Schema.Defect),
52 },
53) {}
54
55class IntegrationWaitTimeoutError extends Schema.TaggedErrorClass<IntegrationWaitTimeoutError>()(
56 "IntegrationWaitTimeoutError",
57 {
58 description: Schema.String,
59 },
60) {}
61
48const sleep = (ms: number) =>62const sleep = (ms: number) =>
49 Effect.promise(() => new Promise<void>((resolve) => setTimeout(resolve, ms)));63 Effect.tryPromise({
64 try: () => new Promise<void>((resolve) => setTimeout(resolve, ms)),
65 catch: (cause) => new IntegrationSleepError({ cause }),
66 }).pipe(Effect.orDie);
50 67
51function waitForSync<A>(68function waitForSync<A>(
52 read: () => A,69 read: () => A,
@@ -63,7 +80,7 @@ function waitForSync<A>( · 10 unmodified lines
63 return value;80 return value;
64 }81 }
65 if (Date.now() >= deadline) {82 if (Date.now() >= deadline) {
66 return yield* Effect.die(new Error(`Timed out waiting for ${description}.`));83 return yield* Effect.die(new IntegrationWaitTimeoutError({ description }));
67 }84 }
68 yield* sleep(10);85 yield* sleep(10);
69 }86 }
apps/server/src/git/Layers/GitManager.test.ts+20−15
@@ -3,10 +3,10 @@ import path from "node:path"; · 2 unmodified lines
3 3
4import * as NodeServices from "@effect/platform-node/NodeServices";4import * as NodeServices from "@effect/platform-node/NodeServices";
5import { it } from "@effect/vitest";5import { it } from "@effect/vitest";
6import { Effect, FileSystem, Layer, PlatformError, Scope } from "effect";6import { Effect, FileSystem, Layer, PlatformError, Schema, Scope } from "effect";
7import { expect } from "vitest";7import { expect } from "vitest";
8 8
9import { GitHubCliError, TextGenerationError } from "../Errors.ts";9import { GitCommandError, GitHubCliError, TextGenerationError } from "../Errors.ts";
10import { type GitManagerShape } from "../Services/GitManager.ts";10import { type GitManagerShape } from "../Services/GitManager.ts";
11import {11import {
12 type GitHubCliShape,12 type GitHubCliShape,
@@ -19,11 +19,16 @@ import { GitService } from "../Services/GitService.ts"; · 6 unmodified lines
19import { GitCoreLive } from "./GitCore.ts";19import { GitCoreLive } from "./GitCore.ts";
20import { makeGitManager } from "./GitManager.ts";20import { makeGitManager } from "./GitManager.ts";
21 21
22class GitFixtureError extends Schema.TaggedErrorClass<GitFixtureError>()("GitFixtureError", {
23 message: Schema.String,
24 cause: Schema.optional(Schema.Defect),
25}) {}
26
22interface FakeGhScenario {27interface FakeGhScenario {
23 prListSequence?: string[];28 prListSequence?: string[];
24 createdPrUrl?: string;29 createdPrUrl?: string;
25 defaultBranch?: string;30 defaultBranch?: string;
26 failWith?: Error;31 failWith?: GitFixtureError;
27}32}
28 33
29interface FakeGitTextGeneration {34interface FakeGitTextGeneration {
@@ -32,7 +37,7 @@ interface FakeGitTextGeneration { · 2 unmodified lines
32 branch: string | null;37 branch: string | null;
33 stagedSummary: string;38 stagedSummary: string;
34 stagedPatch: string;39 stagedPatch: string;
35 }) => Effect.Effect<{ subject: string; body: string }, unknown>;40 }) => Effect.Effect<{ subject: string; body: string }, GitFixtureError>;
36 generatePrContent: (input: {41 generatePrContent: (input: {
37 cwd: string;42 cwd: string;
38 baseBranch: string;43 baseBranch: string;
@@ -40,7 +45,7 @@ interface FakeGitTextGeneration { · 1 unmodified line
40 commitSummary: string;45 commitSummary: string;
41 diffSummary: string;46 diffSummary: string;
42 diffPatch: string;47 diffPatch: string;
43 }) => Effect.Effect<{ title: string; body: string }, unknown>;48 }) => Effect.Effect<{ title: string; body: string }, GitFixtureError>;
44}49}
45 50
46function makeTempDir(51function makeTempDir(
@@ -58,7 +63,7 @@ function runGit( · 11 unmodified lines
58 allowNonZeroExit = false,63 allowNonZeroExit = false,
59): Effect.Effect<64): Effect.Effect<
60 { readonly code: number; readonly stdout: string; readonly stderr: string },65 { readonly code: number; readonly stdout: string; readonly stderr: string },
61 unknown,66 GitCommandError,
62 GitService67 GitService
63> {68> {
64 return Effect.gen(function* () {69 return Effect.gen(function* () {
@@ -74,7 +79,7 @@ function runGit( · 9 unmodified lines
74 79
75function initRepo(80function initRepo(
76 cwd: string,81 cwd: string,
77): Effect.Effect<void, unknown, FileSystem.FileSystem | Scope.Scope | GitService> {82): Effect.Effect<void, PlatformError.PlatformError | GitCommandError, FileSystem.FileSystem | Scope.Scope | GitService> {
78 return Effect.gen(function* () {83 return Effect.gen(function* () {
79 const fs = yield* FileSystem.FileSystem;84 const fs = yield* FileSystem.FileSystem;
80 yield* runGit(cwd, ["init", "--initial-branch=main"]);85 yield* runGit(cwd, ["init", "--initial-branch=main"]);
@@ -88,7 +93,7 @@ function initRepo( · 7 unmodified lines
88 93
89function createBareRemote(): Effect.Effect<94function createBareRemote(): Effect.Effect<
90 string,95 string,
91 unknown,96 PlatformError.PlatformError | GitCommandError,
92 FileSystem.FileSystem | Scope.Scope | GitService97 FileSystem.FileSystem | Scope.Scope | GitService
93> {98> {
94 return Effect.gen(function* () {99 return Effect.gen(function* () {
@@ -139,7 +144,7 @@ function createTextGeneration(overrides: Partial<FakeGitTextGeneration> = {}): T · 44 unmodified lines
139 };144 };
140}145}
141 146
142const normalizeGhError = (error: Error): GitHubCliError => {147const normalizeGhError = (error: GitFixtureError): GitHubCliError => {
143 if (error.message.includes("Command not found: gh")) {148 if (error.message.includes("Command not found: gh")) {
144 return new GitHubCliError({149 return new GitHubCliError({
145 operation: "execute",150 operation: "execute",
@@ -373,7 +378,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 227 unmodified lines
373 378
374 const { manager } = yield* makeManager({379 const { manager } = yield* makeManager({
375 ghScenario: {380 ghScenario: {
376 failWith: new Error("Command not found: gh"),381 failWith: new GitFixtureError({ message: "Command not found: gh" }),
377 },382 },
378 });383 });
379 384
@@ -600,7 +605,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 220 unmodified lines
600 action: "commit_push",605 action: "commit_push",
601 }).pipe(606 }).pipe(
602 Effect.flip,607 Effect.flip,
603 Effect.map((error) => (error instanceof Error ? error.message : String(error))),608 Effect.map((error) => error.message),
604 );609 );
605 expect(errorMessage).toContain("detached HEAD");610 expect(errorMessage).toContain("detached HEAD");
606 }),611 }),
@@ -617,7 +622,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 10 unmodified lines
617 622
618 const { manager } = yield* makeManager({623 const { manager } = yield* makeManager({
619 ghScenario: {624 ghScenario: {
620 failWith: new Error("Command not found: gh"),625 failWith: new GitFixtureError({ message: "Command not found: gh" }),
621 },626 },
622 });627 });
623 628
@@ -626,7 +631,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 2 unmodified lines
626 action: "commit_push_pr",631 action: "commit_push_pr",
627 }).pipe(632 }).pipe(
628 Effect.flip,633 Effect.flip,
629 Effect.map((error) => (error instanceof Error ? error.message : String(error))),634 Effect.map((error) => error.message),
630 );635 );
631 expect(errorMessage).toContain("GitHub CLI (`gh`) is required");636 expect(errorMessage).toContain("GitHub CLI (`gh`) is required");
632 }),637 }),
@@ -643,7 +648,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 10 unmodified lines
643 648
644 const { manager } = yield* makeManager({649 const { manager } = yield* makeManager({
645 ghScenario: {650 ghScenario: {
646 failWith: new Error("authentication failed"),651 failWith: new GitFixtureError({ message: "authentication failed" }),
647 },652 },
648 });653 });
649 654
@@ -652,7 +657,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { · 2 unmodified lines
652 action: "commit_push_pr",657 action: "commit_push_pr",
653 }).pipe(658 }).pipe(
654 Effect.flip,659 Effect.flip,
655 Effect.map((error) => (error instanceof Error ? error.message : String(error))),660 Effect.map((error) => error.message),
656 );661 );
657 expect(errorMessage).toContain("gh auth login");662 expect(errorMessage).toContain("gh auth login");
658 }),663 }),
apps/server/src/keybindings.test.ts+1−7
@@ -211,13 +211,7 @@ it.layer(NodeServices.layer)("keybindings", (it) => { · 210 unmodified lines
211 });211 });
212 212
213 assert.isTrue(configState.keybindings.some((entry) => entry.command === "terminal.toggle"));213 assert.isTrue(configState.keybindings.some((entry) => entry.command === "terminal.toggle"));
214 assert.isFalse(214 assert.isFalse(configState.keybindings.some((entry) => String(entry.command) === "invalid.command"));
215 configState.keybindings.some(
216 (entry) =>
217 // @ts-expect-error - invalid command
218 entry.command === "invalid.command",
219 ),
220 );
221 assert.deepEqual(configState.issues, [215 assert.deepEqual(configState.issues, [
222 {216 {
223 kind: "keybindings.invalid-entry",217 kind: "keybindings.invalid-entry",
apps/server/src/open.ts+4−1
@@ -117,7 +117,10 @@ export const launchDetached = (launch: EditorLaunch) => · 116 unmodified lines
117 });117 });
118 118
119const make = Effect.gen(function* () {119const make = Effect.gen(function* () {
120 const open = yield* Effect.promise(() => import("open"));120 const open = yield* Effect.tryPromise({
121 try: () => import("open"),
122 catch: (cause) => new OpenError({ message: "failed to load browser opener", cause }),
123 });
121 124
122 return {125 return {
123 openBrowser: (target) =>126 openBrowser: (target) =>
apps/server/src/terminal/Layers/NodePTY.ts+17−2
@@ -1,10 +1,18 @@
1import { createRequire } from "node:module";1import { createRequire } from "node:module";
2 2
3import { Effect, FileSystem, Layer, Path } from "effect";3import { Effect, FileSystem, Layer, Path, Schema } from "effect";
4import { PtyAdapter, PtyAdapterShape, PtyExitEvent, PtyProcess } from "../Services/PTY";4import { PtyAdapter, PtyAdapterShape, PtyExitEvent, PtyProcess } from "../Services/PTY";
5 5
6let didEnsureSpawnHelperExecutable = false;6let didEnsureSpawnHelperExecutable = false;
7 7
8export class NodePtyAdapterError extends Schema.TaggedErrorClass<NodePtyAdapterError>()(
9 "NodePtyAdapterError",
10 {
11 message: Schema.String,
12 cause: Schema.optional(Schema.Defect),
13 },
14) {}
15
8const resolveNodePtySpawnHelperPath = Effect.gen(function* () {16const resolveNodePtySpawnHelperPath = Effect.gen(function* () {
9 const requireForNodePty = createRequire(import.meta.url);17 const requireForNodePty = createRequire(import.meta.url);
10 const path = yield* Path.Path;18 const path = yield* Path.Path;
@@ -90,7 +98,14 @@ export const NodePtyAdapterLive = Layer.effect( · 79 unmodified lines
90 const fs = yield* FileSystem.FileSystem;98 const fs = yield* FileSystem.FileSystem;
91 const path = yield* Path.Path;99 const path = yield* Path.Path;
92 100
93 const nodePty = yield* Effect.promise(() => import("node-pty"));101 const nodePty = yield* Effect.tryPromise({
102 try: () => import("node-pty"),
103 catch: (cause) =>
104 new NodePtyAdapterError({
105 message: "failed to load node-pty module",
106 cause,
107 }),
108 });
94 109
95 const ensureNodePtySpawnHelperExecutableCached = yield* Effect.cached(110 const ensureNodePtySpawnHelperExecutableCached = yield* Effect.cached(
96 ensureNodePtySpawnHelperExecutable().pipe(111 ensureNodePtySpawnHelperExecutable().pipe(
apps/server/src/wsServer.ts+7−1
@@ -389,7 +389,13 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< · 388 unmodified lines
389 389
390 case WS_METHODS.projectsSearchEntries: {390 case WS_METHODS.projectsSearchEntries: {
391 const body = stripRequestTag(request.body);391 const body = stripRequestTag(request.body);
392 return yield* Effect.promise(() => searchWorkspaceEntries(body));392 return yield* Effect.tryPromise({
393 try: () => searchWorkspaceEntries(body),
394 catch: (cause) =>
395 new RouteRequestError({
396 message: `Failed to search workspace entries: ${String(cause)}`,
397 }),
398 });
393 }399 }
394 400
395 case WS_METHODS.shellOpenInEditor: {401 case WS_METHODS.shellOpenInEditor: {