Skip to content

Commit 20bdf75

Browse files
authored
feat(fmodata): use Effect.ts internally (#141)
feat(fmodata): use Effect.ts internally for error handling pipelines Replace manual error-threading boilerplate in all builders with Effect pipelines. Effect is used as an internal implementation detail — the public API (Result<T>) is unchanged. Changes: - Add `effect` dependency, remove unused `neverthrow` - Add `src/effect.ts` with bridge utilities (fromResult, makeRequestEffect, runAsResult, tryEffect, fromValidation) - Refactor `_makeRequest` into an Effect pipeline with extracted helpers (_classifyError, _parseHttpError, _checkEmbeddedODataError) - Refactor InsertBuilder.execute() to use Effect.gen pipeline - Refactor UpdateBuilder.execute() to use Effect.gen pipeline - Refactor DeleteBuilder.execute() to use Effect.gen pipeline - Refactor QueryBuilder.execute() to use Effect pipe composition - Refactor BatchBuilder.execute() to use Effect.gen pipeline All 873 tests pass with zero type errors. https://claude.ai/code/session_01VdwR8FRDc9f1qS68z2Sfzo feat(fmodata): add retry policies, validation accumulation, and tracing spans Phase 2 of Effect.ts integration: - Add opt-in RetryPolicy for transient errors (SchemaLockedError, NetworkError, TimeoutError, HTTP 5xx) with exponential backoff and jitter via Effect Schedule - Accumulate all validation errors instead of fail-fast, returning all field issues in a single ValidationError - Add Effect.withSpan tracing to all builder execute() methods and HTTP requests for zero-cost observability All changes remain non-breaking — public API (Result<T>, error classes, type guards) is unchanged. https://claude.ai/code/session_01VdwR8FRDc9f1qS68z2Sfzo feat(fmodata): add Effect services, MockFMServerConnection, and migrate tests Phase 3 of Effect.ts integration: - Define Effect Services (HttpClient, ODataConfig, ODataLogger) for dependency injection - Create MockFMServerConnection with router-style fetch handler and request spy - Export testing utilities via @proofkit/fmodata/testing subpath - Migrate 22 test files from createMockClient/fetchHandler to MockFMServerConnection - All 870 tests passing, 0 type errors, publint passes https://claude.ai/code/session_01VdwR8FRDc9f1qS68z2Sfzo fix(fmodata): limit retries to idempotent HTTP methods fix(fmodata): address remaining PR review findings fix(fmodata): harden write requests and batch response handling refactor(fmodata): migrate all builders to Effect DI via Context/Layer pattern Replace constructor parameter threading (ExecutionContext, databaseName, useEntityIds, etc.) with Effect's Context.Tag + Layer pattern across all builders. Each builder now receives an FMODataLayer and extracts config synchronously via extractConfigFromLayer() for non-Effect methods. Key changes: - All builders (Query, Record, Insert, Update, Delete, Batch) use layer-based DI - SchemaManager and WebhookManager converted to use Effect pipelines - Database.getMetadata/listTableNames/runScript use requestFromService - Removed deprecated makeRequestEffect() and runWithContext() from effect.ts - Simplified ExecutionContext interface to just _getLayer() - Removed unused BuilderConfig interface from shared-types.ts - createDatabaseLayer() enables database-scoped config overrides https://claude.ai/code/session_01EpwtyTQeyjVu3Qykf6aMBd fix(fmodata): resolve DI branch lint regressions <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Route-based testing utilities with request spying and included testing entry in the build * New RetryPolicy type and exported isTransientError helper * **Improvements** * Layered, effect-driven request pipeline with improved error classification and retry support * Centralized runtime/configuration for entity-id and special-column handling; expanded service/type exports * **Tests** * Test suites migrated to use the new mock server and spy-driven patterns <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 3458afb commit 20bdf75

63 files changed

Lines changed: 3777 additions & 3110 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.codex/environments/environment-2.toml

Lines changed: 0 additions & 11 deletions
This file was deleted.

packages/fmodata/package.json

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,24 @@
2020
"default": "./dist/esm/index.js"
2121
}
2222
},
23+
"./testing": {
24+
"import": {
25+
"types": "./dist/esm/testing.d.ts",
26+
"default": "./dist/esm/testing.js"
27+
}
28+
},
29+
"./services": {
30+
"import": {
31+
"types": "./dist/esm/services.d.ts",
32+
"default": "./dist/esm/services.js"
33+
}
34+
},
35+
"./types": {
36+
"import": {
37+
"types": "./dist/esm/types.d.ts",
38+
"default": "./dist/esm/types.js"
39+
}
40+
},
2341
"./package.json": "./package.json"
2442
},
2543
"scripts": {
@@ -48,7 +66,6 @@
4866
"dotenv": "^16.6.1",
4967
"effect": "^3.20.0",
5068
"es-toolkit": "^1.43.0",
51-
"neverthrow": "^8.2.0",
5269
"odata-query": "^8.0.7"
5370
},
5471
"peerDependencies": {

packages/fmodata/skills/fmodata-client/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: >
55
timestampField containerField calcField listField query builder execute() filter operators eq ne gt
66
gte lt lte contains startsWith endsWith matchesPattern inArray notInArray isNull isNotNull and or
77
not tolower toupper trim CRUD insert update delete byId where navigate expand relationships batch
8-
Result error handling neverthrow pattern FMODataError HTTPError ODataError ValidationError
8+
Result error handling Effect.ts pattern FMODataError HTTPError ODataError ValidationError
99
BatchTruncatedError entity IDs FMTID FMFID defaultSelect readValidator writeValidator orderBy asc
1010
desc top skip single maybeSingle count getSingleField FileMaker OData API schema management
1111
webhooks getTableColumns select("all")
Lines changed: 69 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
1+
import { Effect } from "effect";
2+
import { requestFromService, runLayerResult } from "../effect";
3+
import type { FMODataErrorType } from "../errors";
14
import { BatchTruncatedError } from "../errors";
5+
import type { FMODataLayer, ODataConfig } from "../services";
26
import type {
37
BatchItemResult,
48
BatchResult,
59
ExecutableBuilder,
610
ExecuteMethodOptions,
711
ExecuteOptions,
8-
ExecutionContext,
912
Result,
1013
} from "../types";
1114
import { formatBatchRequestFromNative, type ParsedBatchResponse, parseBatchResponse } from "./batch-request";
15+
import { createClientRuntime } from "./runtime";
1216

1317
/**
1418
* Helper type to extract result types from a tuple of ExecutableBuilders.
@@ -64,23 +68,23 @@ function parsedToResponse(parsed: ParsedBatchResponse): Response {
6468
export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
6569
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any ExecutableBuilder result type
6670
private readonly builders: ExecutableBuilder<any>[];
67-
private readonly databaseName: string;
68-
private readonly context: ExecutionContext;
71+
private readonly layer: FMODataLayer;
72+
private readonly config: ODataConfig;
6973

70-
constructor(builders: Builders, databaseName: string, context: ExecutionContext) {
74+
constructor(builders: Builders, layer: FMODataLayer) {
7175
// Convert readonly tuple to mutable array for dynamic additions
7276
this.builders = [...builders];
73-
// Store original tuple for type preservation
74-
this.databaseName = databaseName;
75-
this.context = context;
77+
const runtime = createClientRuntime(layer);
78+
this.layer = runtime.layer;
79+
this.config = runtime.config;
7680
}
7781

7882
/**
7983
* Add a request to the batch dynamically.
8084
* This allows building up batch operations programmatically.
8185
*
8286
* @param builder - An executable builder to add to the batch
83-
* @returns This BatchBuilder for method chaining
87+
* @returns A BatchBuilder typed with the appended request result
8488
* @example
8589
* ```ts
8690
* const batch = db.batch([]);
@@ -89,9 +93,9 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
8993
* const result = await batch.execute();
9094
* ```
9195
*/
92-
addRequest<T>(builder: ExecutableBuilder<T>): this {
96+
addRequest<T>(builder: ExecutableBuilder<T>): BatchBuilder<[...Builders, ExecutableBuilder<T>]> {
9397
this.builders.push(builder);
94-
return this;
98+
return this as unknown as BatchBuilder<[...Builders, ExecutableBuilder<T>]>;
9599
}
96100

97101
/**
@@ -100,19 +104,15 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
100104
*/
101105
// biome-ignore lint/suspicious/noExplicitAny: Request body can be any JSON-serializable value
102106
getRequestConfig(): { method: string; url: string; body?: any } {
103-
// Note: This method is kept for compatibility but batch operations
104-
// should use execute() directly which handles the full Request/Response flow
105107
return {
106108
method: "POST",
107-
url: `/${this.databaseName}/$batch`,
109+
url: `/${this.config.databaseName}/$batch`,
108110
body: undefined, // Body is constructed in execute()
109111
};
110112
}
111113

112114
toRequest(baseUrl: string, _options?: ExecuteOptions): Request {
113-
// Batch operations are not designed to be nested, but we provide
114-
// a basic implementation for interface compliance
115-
const fullUrl = `${baseUrl}/${this.databaseName}/$batch`;
115+
const fullUrl = `${baseUrl}/${this.config.databaseName}/$batch`;
116116
return new Request(fullUrl, {
117117
method: "POST",
118118
headers: {
@@ -124,8 +124,6 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
124124

125125
// biome-ignore lint/suspicious/noExplicitAny: Generic return type for interface compliance
126126
processResponse(_response: Response, _options?: ExecuteOptions): Promise<Result<any>> {
127-
// This should not typically be called for batch operations
128-
// as they handle their own response processing
129127
return Promise.resolve({
130128
data: undefined,
131129
error: {
@@ -137,6 +135,28 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
137135
});
138136
}
139137

138+
/**
139+
* Creates a failed BatchResult where all operations are marked as failed with the given error.
140+
*/
141+
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
142+
private failAllResults(error: any): BatchResult<ExtractTupleTypes<Builders>> {
143+
const errorCount = this.builders.length;
144+
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
145+
const results: BatchItemResult<any>[] = this.builders.map(() => ({
146+
data: undefined,
147+
error,
148+
status: 0,
149+
}));
150+
return {
151+
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for complex generic return type
152+
results: results as any,
153+
successCount: 0,
154+
errorCount,
155+
truncated: false,
156+
firstErrorIndex: errorCount > 0 ? 0 : null,
157+
};
158+
}
159+
140160
/**
141161
* Execute the batch operation.
142162
*
@@ -146,41 +166,25 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
146166
async execute<EO extends ExecuteOptions>(
147167
options?: ExecuteMethodOptions<EO>,
148168
): Promise<BatchResult<ExtractTupleTypes<Builders>>> {
149-
const baseUrl = this.context._getBaseUrl?.();
169+
const baseUrl = this.config.baseUrl;
150170
if (!baseUrl) {
151-
// Return BatchResult with all operations marked as failed
152-
const errorCount = this.builders.length;
153-
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
154-
const results: BatchItemResult<any>[] = this.builders.map((_, _i) => ({
155-
data: undefined,
156-
error: {
157-
name: "ConfigurationError",
158-
message: "Base URL not available - execution context must implement _getBaseUrl()",
159-
timestamp: new Date(),
160-
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for error object
161-
} as any,
162-
status: 0,
163-
}));
164-
165-
return {
166-
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for complex generic return type
167-
results: results as any,
168-
successCount: 0,
169-
errorCount,
170-
truncated: false,
171-
firstErrorIndex: 0,
172-
};
171+
return this.failAllResults({
172+
name: "ConfigurationError",
173+
message: "Base URL not available in ODataConfig",
174+
timestamp: new Date(),
175+
});
173176
}
174177

175-
try {
176-
// Convert builders to native Request objects
178+
const pipeline = Effect.gen(this, function* () {
179+
// Step 1: Convert builders to Request objects and format batch
177180
const requests: Request[] = this.builders.map((builder) => builder.toRequest(baseUrl, options));
181+
const { body, boundary } = yield* Effect.tryPromise({
182+
try: () => formatBatchRequestFromNative(requests, baseUrl),
183+
catch: (e) => e as FMODataErrorType,
184+
});
178185

179-
// Format batch request (automatically groups mutations into changesets)
180-
const { body, boundary } = await formatBatchRequestFromNative(requests, baseUrl);
181-
182-
// Execute the batch request
183-
const response = await this.context._makeRequest<string>(`/${this.databaseName}/$batch`, {
186+
// Step 2: Execute the batch HTTP request via DI
187+
const responseData = yield* requestFromService<string>(`/${this.config.databaseName}/$batch`, {
184188
...options,
185189
method: "POST",
186190
headers: {
@@ -191,53 +195,25 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
191195
body,
192196
});
193197

194-
if (response.error) {
195-
// Return BatchResult with all operations marked as failed
196-
const errorCount = this.builders.length;
197-
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
198-
const results: BatchItemResult<any>[] = this.builders.map((_, _i) => ({
199-
data: undefined,
200-
error: response.error,
201-
status: 0,
202-
}));
203-
204-
return {
205-
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for complex generic return type
206-
results: results as any,
207-
successCount: 0,
208-
errorCount,
209-
truncated: false,
210-
firstErrorIndex: 0,
211-
};
212-
}
213-
214-
// Extract the actual boundary from the response
215-
// FileMaker uses its own boundary, not the one we sent
216-
const firstLine = response.data.split("\r\n")[0] || response.data.split("\n")[0] || "";
198+
// Step 3: Parse multipart response
199+
const firstLine = responseData.split("\r\n")[0] || responseData.split("\n")[0] || "";
217200
const actualBoundary = firstLine.startsWith("--") ? firstLine.substring(2) : boundary;
218-
219-
// Parse the multipart response
220201
const contentTypeHeader = `multipart/mixed; boundary=${actualBoundary}`;
221-
const parsedResponses = parseBatchResponse(response.data, contentTypeHeader);
222-
223-
// Process each response using the corresponding builder
224-
// Build BatchResult with per-item results
225-
type _ResultTuple = ExtractTupleTypes<Builders>;
202+
const parsedResponses = parseBatchResponse(responseData, contentTypeHeader);
226203

204+
// Step 4: Process each response using the corresponding builder
227205
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
228206
const results: BatchItemResult<any>[] = [];
229207
let successCount = 0;
230208
let errorCount = 0;
231209
let firstErrorIndex: number | null = null;
232210
const truncated = parsedResponses.length < this.builders.length;
233211

234-
// Process builders sequentially to preserve tuple order
235212
for (let i = 0; i < this.builders.length; i++) {
236213
const builder = this.builders[i];
237214
const parsed = parsedResponses[i];
238215

239216
if (!parsed) {
240-
// Truncated - operation never executed
241217
const failedAtIndex = firstErrorIndex ?? i;
242218
results.push({
243219
data: undefined,
@@ -249,7 +225,6 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
249225
}
250226

251227
if (!builder) {
252-
// Should not happen, but handle gracefully
253228
results.push({
254229
data: undefined,
255230
error: {
@@ -267,28 +242,20 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
267242
continue;
268243
}
269244

270-
// Convert parsed response to native Response
271245
const nativeResponse = parsedToResponse(parsed);
272-
273-
// Let the builder process its own response
274-
const result = await builder.processResponse(nativeResponse, options);
246+
const result = yield* Effect.tryPromise({
247+
try: () => builder.processResponse(nativeResponse, options),
248+
catch: (e) => e as FMODataErrorType,
249+
});
275250

276251
if (result.error) {
277-
results.push({
278-
data: undefined,
279-
error: result.error,
280-
status: parsed.status,
281-
});
252+
results.push({ data: undefined, error: result.error, status: parsed.status });
282253
errorCount++;
283254
if (firstErrorIndex === null) {
284255
firstErrorIndex = i;
285256
}
286257
} else {
287-
results.push({
288-
data: result.data,
289-
error: undefined,
290-
status: parsed.status,
291-
});
258+
results.push({ data: result.data, error: undefined, status: parsed.status });
292259
successCount++;
293260
}
294261
}
@@ -300,30 +267,14 @@ export class BatchBuilder<Builders extends readonly ExecutableBuilder<any>[]> {
300267
errorCount,
301268
truncated,
302269
firstErrorIndex,
303-
};
304-
} catch (err) {
305-
// On exception, return a BatchResult with all operations marked as failed
306-
const errorCount = this.builders.length;
307-
// biome-ignore lint/suspicious/noExplicitAny: Generic constraint accepting any result type
308-
const results: BatchItemResult<any>[] = this.builders.map((_, _i) => ({
309-
data: undefined,
310-
error: {
311-
name: "BatchError",
312-
message: err instanceof Error ? err.message : "Unknown error",
313-
timestamp: new Date(),
314-
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for error object
315-
} as any,
316-
status: 0,
317-
}));
270+
} as BatchResult<ExtractTupleTypes<Builders>>;
271+
});
318272

319-
return {
320-
// biome-ignore lint/suspicious/noExplicitAny: Type assertion for complex generic return type
321-
results: results as any,
322-
successCount: 0,
323-
errorCount,
324-
truncated: false,
325-
firstErrorIndex: 0,
326-
};
273+
// For batch, errors at the transport level fail all operations
274+
const result = await runLayerResult(this.layer, pipeline, "fmodata.batch");
275+
if (result.error) {
276+
return this.failAllResults(result.error);
327277
}
278+
return result.data;
328279
}
329280
}

0 commit comments

Comments
 (0)