From 7f855819934cc0d2f452400bece03b7d9046bb7a Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:39:43 +0100 Subject: [PATCH 1/6] Make resource operation outcomes explicit --- .changeset/resource-operation-outcomes.md | 9 ++ docs/internals/resource.md | 24 ++-- examples/reconciler/src/static-site.ts | 11 +- .../aws.iac/src/resources/api-gateway/api.ts | 28 +++- .../aws.iac/src/resources/api-gateway/auth.ts | 22 +++- .../api-gateway/lambda-integration.ts | 20 ++- .../src/resources/api-gateway/route.ts | 24 +++- .../src/resources/api-gateway/stage.ts | 20 ++- .../src/resources/event-bridge/rule.ts | 82 +++++++----- .../src/resources/lambda/lambda-role.ts | 21 ++- .../aws.iac/src/resources/lambda/lambda.ts | 119 ++++++++++------- packages/core/src/orchestrator/resource.ts | 11 +- .../test/orchestrator/resource.doubles.ts | 9 +- .../test/provisioner/operation.create.test.ts | 11 +- .../src/operations/operation.create.ts | 15 ++- .../src/operations/operation.delete.ts | 38 ++---- .../src/operations/operation.read.ts | 72 +++++----- .../src/operations/operation.types.ts | 32 +---- .../src/operations/operation.update.ts | 15 ++- packages/reconciler/src/plan.ts | 17 +-- packages/reconciler/src/reconciler.ts | 27 ++-- .../test/operation.workflows.test.ts | 123 ++++++++++-------- .../reconciler/test/reconciler.deploy.test.ts | 41 +++--- .../reconciler/test/reconciler.plan.test.ts | 65 ++++++--- packages/resource/src/resource.ts | 52 +++----- packages/resource/test/resource.doubles.ts | 9 +- packages/std.iac/src/resources/fs/file.ts | 21 ++- packages/std.iac/src/resources/fs/zip.ts | 28 +++- 28 files changed, 562 insertions(+), 404 deletions(-) create mode 100644 .changeset/resource-operation-outcomes.md diff --git a/.changeset/resource-operation-outcomes.md b/.changeset/resource-operation-outcomes.md new file mode 100644 index 0000000..580b357 --- /dev/null +++ b/.changeset/resource-operation-outcomes.md @@ -0,0 +1,9 @@ +--- +"@notation/aws.iac": minor +"@notation/core": minor +"@notation/reconciler": minor +"@notation/resource": minor +"@notation/std.iac": minor +--- + +Replace error matcher declarations and read retry conditions with explicit resource read outcomes and retryable resource errors. diff --git a/docs/internals/resource.md b/docs/internals/resource.md index 4cf0921..8afd5f1 100644 --- a/docs/internals/resource.md +++ b/docs/internals/resource.md @@ -137,19 +137,17 @@ All schema items carry these fields: ## Operations -`defineOperations` accepts CRUD handlers and error-handling configuration: - -| Field | Required | Signature / Description | -| ---------------------- | -------- | ------------------------------------------------------------------------------------------------------- | -| `create` | yes | `(params: Params) => Promise>` – create the resource, return its computed key. | -| `read` | no | `(key: CompoundKey) => Promise>` – read current state. | -| `update` | no | `(key, patch, params, state) => Promise` – apply a partial update. | -| `delete` | yes | `(key, state) => Promise` – destroy the resource. | -| `deriveParams` | no | Computes intrinsic derived params from config (not dependency-aware). | -| `retryReadOnCondition` | no | Conditions on read output that trigger a retry (e.g. eventual consistency). | -| `failOnError` | no | Error matchers that cause immediate failure with a reason. | -| `notFoundOnError` | no | Error matchers that indicate the resource does not exist. | -| `retryLaterOnError` | no | Error matchers that indicate a transient failure worth retrying. | +`defineOperations` accepts CRUD handlers and parameter derivation: + +| Field | Required | Signature / Description | +| -------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `create` | yes | `(params: Params) => Promise>` – create the resource, return its computed key. | +| `read` | no | `(key: CompoundKey) => Promise>>` – report the remote as `found`, `absent`, or temporarily `pending`. | +| `update` | no | `(key, patch, params, state) => Promise` – apply a partial update. | +| `delete` | yes | `(key, state) => Promise` – ensure the resource is absent. Implementations must also succeed when the remote resource is already gone. | +| `deriveParams` | no | Computes intrinsic derived params from config (not dependency-aware). | + +Resource operations translate provider-specific responses at the provider boundary. A read returns `{ status: "found", output }`, `{ status: "absent" }`, or `{ status: "pending", reason }`. A mutation throws `RetryableResourceError` when the provider explicitly reports a transient condition; all other errors fail the operation. ## Dependencies diff --git a/examples/reconciler/src/static-site.ts b/examples/reconciler/src/static-site.ts index 297a00c..5b30db0 100644 --- a/examples/reconciler/src/static-site.ts +++ b/examples/reconciler/src/static-site.ts @@ -9,10 +9,6 @@ type StaticSiteApi = { ReadResult: { html: string }; }; -class SiteNotFound extends Error { - readonly name = "SiteNotFound"; -} - const staticSite = resource({ type: "local/site/static" }); export const StaticSite = staticSite @@ -38,9 +34,9 @@ export const StaticSite = staticSite path.join(siteDirectory, "index.html"), "utf8", ); - return { html }; + return { status: "found", output: { html } } as const; } catch (error) { - if (isFileMissing(error)) throw new SiteNotFound(siteDirectory); + if (isFileMissing(error)) return { status: "absent" } as const; throw error; } }, @@ -48,9 +44,8 @@ export const StaticSite = staticSite await writeFile(path.join(siteDirectory, "index.html"), html, "utf8"); }, delete: async ({ siteDirectory }) => { - await rm(siteDirectory, { recursive: true }); + await rm(siteDirectory, { recursive: true, force: true }); }, - notFoundOnError: [{ name: "SiteNotFound", reason: "site was removed" }], }); function isFileMissing(error: unknown): boolean { diff --git a/packages/aws.iac/src/resources/api-gateway/api.ts b/packages/aws.iac/src/resources/api-gateway/api.ts index 712cc4e..b0a323e 100644 --- a/packages/aws.iac/src/resources/api-gateway/api.ts +++ b/packages/aws.iac/src/resources/api-gateway/api.ts @@ -97,19 +97,33 @@ export const Api = apiSchema.defineOperations({ return { ApiId: result.ApiId! }; }, async read(key) { - const command = new sdk.GetApiCommand(key); - const result = await apiGatewayClient.send(command); - // todo: check types or correct or if RouteKey is actually in result - // if not, need to pass the original params to read - return { RouteKey: "", ...result }; + try { + const command = new sdk.GetApiCommand(key); + const result = await apiGatewayClient.send(command); + // todo: check types or correct or if RouteKey is actually in result + // if not, need to pass the original params to read + return { + status: "found", + output: { RouteKey: "", ...result }, + } as const; + } catch (error) { + if (error instanceof sdk.NotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, async update(key, params) { const command = new sdk.UpdateApiCommand({ ...key, ...params }); await apiGatewayClient.send(command); }, async delete(pk) { - const command = new sdk.DeleteApiCommand(pk); - await apiGatewayClient.send(command); + try { + const command = new sdk.DeleteApiCommand(pk); + await apiGatewayClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NotFoundException)) throw error; + } }, }); diff --git a/packages/aws.iac/src/resources/api-gateway/auth.ts b/packages/aws.iac/src/resources/api-gateway/auth.ts index a56996b..a4debaa 100644 --- a/packages/aws.iac/src/resources/api-gateway/auth.ts +++ b/packages/aws.iac/src/resources/api-gateway/auth.ts @@ -59,18 +59,28 @@ export const RouteAuth = apiSchema }; }, read: async (key) => { - const command = new sdk.GetAuthorizerCommand(key); - const result = await apiGatewayClient.send(command); - - return result; + try { + const command = new sdk.GetAuthorizerCommand(key); + const output = await apiGatewayClient.send(command); + return { status: "found", output } as const; + } catch (error) { + if (error instanceof sdk.NotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, patch, params) => { const command = new sdk.UpdateAuthorizerCommand({ ...key, ...params }); await apiGatewayClient.send(command); }, delete: async (params) => { - const command = new sdk.DeleteAuthorizerCommand(params); - await apiGatewayClient.send(command); + try { + const command = new sdk.DeleteAuthorizerCommand(params); + await apiGatewayClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NotFoundException)) throw error; + } }, }) .requireDependencies() diff --git a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts index 9390380..e34bfa6 100644 --- a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts +++ b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts @@ -118,16 +118,28 @@ export const LambdaIntegration = integrationSchema return { IntegrationId: result.IntegrationId! }; }, read: async (key) => { - const command = new sdk.GetIntegrationCommand(key); - return apiGatewayClient.send(command); + try { + const command = new sdk.GetIntegrationCommand(key); + const output = await apiGatewayClient.send(command); + return { status: "found", output } as const; + } catch (error) { + if (error instanceof sdk.NotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, params) => { const command = new sdk.UpdateIntegrationCommand({ ...key, ...params }); await apiGatewayClient.send(command); }, delete: async (key) => { - const command = new sdk.DeleteIntegrationCommand(key); - await apiGatewayClient.send(command); + try { + const command = new sdk.DeleteIntegrationCommand(key); + await apiGatewayClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NotFoundException)) throw error; + } }, }) .requireDependencies() diff --git a/packages/aws.iac/src/resources/api-gateway/route.ts b/packages/aws.iac/src/resources/api-gateway/route.ts index e381f04..55a6661 100644 --- a/packages/aws.iac/src/resources/api-gateway/route.ts +++ b/packages/aws.iac/src/resources/api-gateway/route.ts @@ -92,17 +92,31 @@ export const Route = routeSchema return { RouteId: result.RouteId! }; }, read: async (key) => { - const command = new sdk.GetRouteCommand(key); - const result = await apiGatewayClient.send(command); - return { ...key, ...result }; + try { + const command = new sdk.GetRouteCommand(key); + const result = await apiGatewayClient.send(command); + return { + status: "found", + output: { ...key, ...result }, + } as const; + } catch (error) { + if (error instanceof sdk.NotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, patch, params) => { const command = new sdk.UpdateRouteCommand({ ...key, ...params }); await apiGatewayClient.send(command); }, delete: async (key) => { - const command = new sdk.DeleteRouteCommand(key); - await apiGatewayClient.send(command); + try { + const command = new sdk.DeleteRouteCommand(key); + await apiGatewayClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NotFoundException)) throw error; + } }, }) .requireDependencies() diff --git a/packages/aws.iac/src/resources/api-gateway/stage.ts b/packages/aws.iac/src/resources/api-gateway/stage.ts index c253b69..65d3c1d 100644 --- a/packages/aws.iac/src/resources/api-gateway/stage.ts +++ b/packages/aws.iac/src/resources/api-gateway/stage.ts @@ -76,16 +76,28 @@ export const Stage = stageSchema await apiGatewayClient.send(command); }, read: async (key) => { - const command = new sdk.GetStageCommand(key); - return apiGatewayClient.send(command); + try { + const command = new sdk.GetStageCommand(key); + const output = await apiGatewayClient.send(command); + return { status: "found", output } as const; + } catch (error) { + if (error instanceof sdk.NotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, params) => { const command = new sdk.UpdateStageCommand({ ...key, ...params }); await apiGatewayClient.send(command); }, delete: async (key) => { - const command = new sdk.DeleteStageCommand(key); - await apiGatewayClient.send(command); + try { + const command = new sdk.DeleteStageCommand(key); + await apiGatewayClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NotFoundException)) throw error; + } }, }) .requireDependencies() diff --git a/packages/aws.iac/src/resources/event-bridge/rule.ts b/packages/aws.iac/src/resources/event-bridge/rule.ts index a2d1150..107a385 100644 --- a/packages/aws.iac/src/resources/event-bridge/rule.ts +++ b/packages/aws.iac/src/resources/event-bridge/rule.ts @@ -60,21 +60,32 @@ const eventBridgeRuleSchema = eventBridgeRule.defineSchema({ export const EventBridgeRule = eventBridgeRuleSchema .defineOperations({ read: async (key) => { - const describeRuleCommand = new sdk.DescribeRuleCommand(key); - const listRuleTargetsCommand = new sdk.ListTargetsByRuleCommand({ - Rule: key.Name, - EventBusName: key.EventBusName, - }); - - const [ruleDescriptionResult, listRuleTargetsResult] = await Promise.all([ - eventBridgeClient.send(describeRuleCommand), - eventBridgeClient.send(listRuleTargetsCommand), - ]); + try { + const describeRuleCommand = new sdk.DescribeRuleCommand(key); + const listRuleTargetsCommand = new sdk.ListTargetsByRuleCommand({ + Rule: key.Name, + EventBusName: key.EventBusName, + }); - return { - ...ruleDescriptionResult, - ...listRuleTargetsResult, - }; + const [ruleDescriptionResult, listRuleTargetsResult] = + await Promise.all([ + eventBridgeClient.send(describeRuleCommand), + eventBridgeClient.send(listRuleTargetsCommand), + ]); + + return { + status: "found", + output: { + ...ruleDescriptionResult, + ...listRuleTargetsResult, + }, + } as const; + } catch (error) { + if (error instanceof sdk.ResourceNotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, create: async (params) => { @@ -102,27 +113,30 @@ export const EventBridgeRule = eventBridgeRuleSchema await eventBridgeClient.send(updateTargetsCommand); }, delete: async (key) => { - // The targets must be deleted first, otherwise the API will return an error response - - const existingTargets = await eventBridgeClient.send( - new sdk.ListTargetsByRuleCommand({ - Rule: key.Name, - EventBusName: key.EventBusName, - }), - ); - - if (existingTargets.Targets && existingTargets.Targets.length > 0) { - const deleteTargetsCommand = new sdk.RemoveTargetsCommand({ - Rule: key.Name, - EventBusName: key.EventBusName, - Ids: existingTargets.Targets.map((target) => target.Id!), - }); - - await eventBridgeClient.send(deleteTargetsCommand); + try { + // The targets must be deleted first, otherwise the API will return an error response + const existingTargets = await eventBridgeClient.send( + new sdk.ListTargetsByRuleCommand({ + Rule: key.Name, + EventBusName: key.EventBusName, + }), + ); + + if (existingTargets.Targets && existingTargets.Targets.length > 0) { + const deleteTargetsCommand = new sdk.RemoveTargetsCommand({ + Rule: key.Name, + EventBusName: key.EventBusName, + Ids: existingTargets.Targets.map((target) => target.Id!), + }); + + await eventBridgeClient.send(deleteTargetsCommand); + } + + const deleteRuleCommand = new sdk.DeleteRuleCommand(key); + await eventBridgeClient.send(deleteRuleCommand); + } catch (error) { + if (!(error instanceof sdk.ResourceNotFoundException)) throw error; } - - const deleteRuleCommand = new sdk.DeleteRuleCommand(key); - await eventBridgeClient.send(deleteRuleCommand); }, }) .requireDependencies() diff --git a/packages/aws.iac/src/resources/lambda/lambda-role.ts b/packages/aws.iac/src/resources/lambda/lambda-role.ts index 51c5f39..18843d1 100644 --- a/packages/aws.iac/src/resources/lambda/lambda-role.ts +++ b/packages/aws.iac/src/resources/lambda/lambda-role.ts @@ -69,17 +69,28 @@ export const LambdaIamRole = lambdaIamRoleSchema.defineOperations({ await iamClient.send(command); }, read: async (key) => { - const command = new sdk.GetRoleCommand(key); - const { Role } = await iamClient.send(command); - return Role!; + try { + const command = new sdk.GetRoleCommand(key); + const { Role } = await iamClient.send(command); + return { status: "found", output: Role! } as const; + } catch (error) { + if (error instanceof sdk.NoSuchEntityException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, params) => { const command = new sdk.UpdateRoleCommand({ ...key, ...params }); await iamClient.send(command); }, delete: async (key) => { - const command = new sdk.DeleteRoleCommand(key); - await iamClient.send(command); + try { + const command = new sdk.DeleteRoleCommand(key); + await iamClient.send(command); + } catch (error) { + if (!(error instanceof sdk.NoSuchEntityException)) throw error; + } }, deriveParams: () => ({ AssumeRolePolicyDocument: JSON.stringify(lambdaTrustPolicy), diff --git a/packages/aws.iac/src/resources/lambda/lambda.ts b/packages/aws.iac/src/resources/lambda/lambda.ts index 12d75a2..97e4582 100644 --- a/packages/aws.iac/src/resources/lambda/lambda.ts +++ b/packages/aws.iac/src/resources/lambda/lambda.ts @@ -1,4 +1,4 @@ -import { resource, typed } from "@notation/resource"; +import { resource, RetryableResourceError, typed } from "@notation/resource"; import * as sdk from "@aws-sdk/client-lambda"; import { lambdaClient } from "src/utils/aws-clients"; import { AwsSchema } from "src/utils/types"; @@ -186,7 +186,7 @@ export const LambdaFunction = lambdaFunctionSchema Code: { ZipFile: params.Code.ZipFile }, }); - await lambdaClient.send(command); + await runLambdaMutation(() => lambdaClient.send(command)); // if (params.ReservedConcurrentExecutions) { // const concurrencyCommand = new sdk.PutFunctionConcurrencyCommand({ @@ -198,21 +198,44 @@ export const LambdaFunction = lambdaFunctionSchema }, read: async (key) => { - const command = new sdk.GetFunctionCommand(key); - const { Code, Configuration, Concurrency } = - await lambdaClient.send(command); + try { + const command = new sdk.GetFunctionCommand(key); + const { Code, Configuration, Concurrency } = + await lambdaClient.send(command); - return { - ...Configuration, - Layers: Configuration!.Layers?.map((layer) => layer.Arn), - ...Concurrency, - Code: { - S3Bucket: Code?.Location?.split("/")[0], - S3Key: Code?.Location?.split("/")[1], - S3ObjectVersion: Code?.Location?.split("/")[2], - ZipFile: undefined, - }, - }; + if (Configuration?.State !== "Active") { + return { + status: "pending", + reason: "Waiting for Lambda to become active", + } as const; + } + if (!Configuration.RevisionId) { + return { + status: "pending", + reason: "Waiting for Lambda to be deployed", + } as const; + } + + return { + status: "found", + output: { + ...Configuration, + Layers: Configuration.Layers?.map((layer) => layer.Arn), + ...Concurrency, + Code: { + S3Bucket: Code?.Location?.split("/")[0], + S3Key: Code?.Location?.split("/")[1], + S3ObjectVersion: Code?.Location?.split("/")[2], + ZipFile: undefined, + }, + }, + } as const; + } catch (error) { + if (error instanceof sdk.ResourceNotFoundException) { + return { status: "absent" } as const; + } + throw error; + } }, update: async (key, patch, params) => { @@ -226,7 +249,7 @@ export const LambdaFunction = lambdaFunctionSchema ...key, ...conf, }); - await lambdaClient.send(confCommand); + await runLambdaMutation(() => lambdaClient.send(confCommand)); } if (CodeSha256) { @@ -234,39 +257,18 @@ export const LambdaFunction = lambdaFunctionSchema ...key, ZipFile: params.Code.ZipFile, }); - await lambdaClient.send(codeCommand); + await runLambdaMutation(() => lambdaClient.send(codeCommand)); } }, delete: async (key) => { - const command = new sdk.DeleteFunctionCommand(key); - await lambdaClient.send(command); + try { + const command = new sdk.DeleteFunctionCommand(key); + await runLambdaMutation(() => lambdaClient.send(command)); + } catch (error) { + if (!(error instanceof sdk.ResourceNotFoundException)) throw error; + } }, - retryLaterOnError: [ - { - name: "InvalidParameterValueException", - message: - "The role defined for the function cannot be assumed by Lambda.", - reason: "Waiting for IAM role to propagate", - }, - { - name: "InvalidParameterValueException", - message: "The provided execution role does not have permissions", - // todo: find real reason this is here - reason: "Waiting for IAM role to propagate", - }, - ], - retryReadOnCondition: [ - { - key: "State", - value: "Active", - reason: "Waiting for lambda to become active", - }, - { - key: "RevisionId", - reason: "Waiting for lambda to be deployed", - }, - ], }) .requireDependencies() .deriveParams(async ({ deps }) => ({ @@ -278,6 +280,33 @@ export const LambdaFunction = lambdaFunctionSchema export type LambdaFunctionInstance = InstanceType; +async function runLambdaMutation(mutation: () => Promise): Promise { + try { + return await mutation(); + } catch (error) { + if (isIamPropagationFailure(error)) { + throw new RetryableResourceError("Waiting for IAM role to propagate", { + cause: error, + }); + } + throw error; + } +} + +function isIamPropagationFailure( + error: unknown, +): error is sdk.InvalidParameterValueException { + if (!(error instanceof sdk.InvalidParameterValueException)) return false; + return ( + error.message.startsWith( + "The role defined for the function cannot be assumed by Lambda.", + ) || + error.message.startsWith( + "The provided execution role does not have permissions", + ) + ); +} + export type LambdaFunctionConfig = ConstructorParameters< typeof LambdaFunction >[0]["config"]; diff --git a/packages/core/src/orchestrator/resource.ts b/packages/core/src/orchestrator/resource.ts index 58dcf81..e62eba6 100644 --- a/packages/core/src/orchestrator/resource.ts +++ b/packages/core/src/orchestrator/resource.ts @@ -1,9 +1,7 @@ export type { BaseResource, ResourceType, - ErrorMatcher, - ResultCondition, - ResultConditions, + ResourceReadResult, ResourceOpts, DefineResourceMeta, DefineResourceApiSchema, @@ -16,4 +14,9 @@ export type { SchemaItem, } from "@notation/resource"; -export { Resource, defineResource, resource } from "@notation/resource"; +export { + Resource, + RetryableResourceError, + defineResource, + resource, +} from "@notation/resource"; diff --git a/packages/core/test/orchestrator/resource.doubles.ts b/packages/core/test/orchestrator/resource.doubles.ts index 5c7f10e..f086c66 100644 --- a/packages/core/test/orchestrator/resource.doubles.ts +++ b/packages/core/test/orchestrator/resource.doubles.ts @@ -65,7 +65,14 @@ export const testOperations = { async delete() {}, async update() {}, async read() { - return { primaryKey: "", optionalSecondaryKey: "", requiredParam: "" }; + return { + status: "found", + output: { + primaryKey: "", + optionalSecondaryKey: "", + requiredParam: "", + }, + } as const; }, deriveParams() { return { intrinsicParam: true }; diff --git a/packages/core/test/provisioner/operation.create.test.ts b/packages/core/test/provisioner/operation.create.test.ts index 2de482d..f0b990d 100644 --- a/packages/core/test/provisioner/operation.create.test.ts +++ b/packages/core/test/provisioner/operation.create.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { createResourceOperation, createStepRunner, runOperation } from "@notation/reconciler"; +import { + createResourceOperation, + createStepRunner, + runOperation, +} from "@notation/reconciler"; import { MemoryStateBackend } from "@notation/state"; import { TestResourceSchema, @@ -13,7 +17,10 @@ describe("resource creation", () => { const stateBackend = new MemoryStateBackend(); const readResult = { ...testResourceOutput, volatileComputed: "123" }; const createMock = vi.fn(async () => ({ primaryKey: "" })); - const readMock = vi.fn(async () => readResult); + const readMock = vi.fn(async () => ({ + status: "found" as const, + output: readResult, + })); const TestResource = TestResourceSchema.defineOperations({ ...testOperations, diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index 98eac89..ecb6038 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -1,11 +1,11 @@ import { RetryableError, createWorkflow } from "yieldstar"; +import { RetryableResourceError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type CreateResourceParams, type StepRunner, emitLifecycleEvent, getErrorDetails, - matchError, } from "./operation.types"; import { readResourceOperation } from "./operation.read"; @@ -29,9 +29,8 @@ export async function* createResourceOperation( try { return await params.resource.create(resourceParams); } catch (err) { - const matcher = matchError(err, params.resource.retryLaterOnError); - if (matcher) { - throw new RetryableError(matcher.reason, { + if (err instanceof RetryableResourceError) { + throw new RetryableError(err.message, { ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), }); } @@ -52,11 +51,17 @@ export async function* createResourceOperation( state: params.state, emit: params.emit, readPollOptions: params.readPollOptions, + retryAbsent: true, }); + if (readResult.status !== "found") { + throw new Error( + "Post-create read completed without finding the resource", + ); + } params.resource.setOutput({ ...params.resource.output, - ...readResult, + ...readResult.output, }); yield* step.run("create:persist-state", async () => { diff --git a/packages/reconciler/src/operations/operation.delete.ts b/packages/reconciler/src/operations/operation.delete.ts index ffa5de4..39a446a 100644 --- a/packages/reconciler/src/operations/operation.delete.ts +++ b/packages/reconciler/src/operations/operation.delete.ts @@ -1,11 +1,11 @@ import { RetryableError, createWorkflow } from "yieldstar"; +import { RetryableResourceError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type DeleteResourceParams, type StepRunner, emitLifecycleEvent, getErrorDetails, - matchError, } from "./operation.types"; export async function* deleteResourceOperation( @@ -20,33 +20,21 @@ export async function* deleteResourceOperation( } try { - try { - yield* step.run("delete:remote", async () => { - try { - await params.resource.delete( - params.resource.key, - params.resource.toState(params.resource.output), - ); - } catch (err) { - const matcher = matchError(err, params.resource.retryLaterOnError); - if (matcher) { - throw new RetryableError(matcher.reason, { - ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), - }); - } - throw err; + yield* step.run("delete:remote", async () => { + try { + await params.resource.delete( + params.resource.key, + params.resource.toState(params.resource.output), + ); + } catch (err) { + if (err instanceof RetryableResourceError) { + throw new RetryableError(err.message, { + ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), + }); } - }); - } catch (err) { - const matcher = matchError(err, params.resource.notFoundOnError); - if (matcher) { - await emitLifecycleEvent(params, "delete", "skip", { - reason: matcher.reason, - }); - } else { throw err; } - } + }); yield* step.run("delete:persist-state", () => params.state.delete(params.resource.id, params.expectedRev), diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 79db1de..8eaf630 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -1,4 +1,4 @@ -import { createWorkflow } from "yieldstar"; +import { RetryableError, createWorkflow } from "yieldstar"; import { DEFAULT_READ_POLL_OPTIONS, type ReadResourceParams, @@ -7,35 +7,18 @@ import { getErrorDetails, } from "./operation.types"; -type ReadRetryCondition = { - key: string; - reason: string; - value?: unknown; -}; - -function needsReadRetry( - readResult: Record, - retryConditions: ReadonlyArray, -) { - return retryConditions.find((condition) => { - const resultValue = readResult[condition.key]; - if (condition.value !== undefined) { - return resultValue !== condition.value; - } - - return !resultValue; - }); -} +export type SettledResourceReadResult = + { status: "found"; output: Record } | { status: "absent" }; export async function* readResourceOperation( step: StepRunner, params: ReadResourceParams, -): AsyncGenerator, unknown> { +): AsyncGenerator { await emitLifecycleEvent(params, "read", "start"); if (params.dryRun) { await emitLifecycleEvent(params, "read", "dry-run"); - return {}; + return { status: "found", output: {} }; } try { @@ -55,36 +38,41 @@ export async function* readResourceOperation( reason: "read-not-implemented", }); await emitLifecycleEvent(params, "read", "success"); - return merged as Record; + return { + status: "found", + output: merged as Record, + }; } - let remoteOutput: Record = {}; - const retryConditions = (params.resource.retryReadOnCondition ?? []).filter( - Boolean, - ) as ReadRetryCondition[]; + const remote = yield* step.run("read:remote", async () => { + const result = await params.resource.read!(params.resource.key); + if (result.status === "pending") { + throw new RetryableError(result.reason, { + ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + if (result.status === "absent" && params.retryAbsent) { + throw new RetryableError("Waiting for resource to become visible", { + ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + return result; + }); - if (retryConditions.length > 0) { - yield* step.poll( - "read:poll-until-settled", - params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS, - async () => { - remoteOutput = await params.resource.read!(params.resource.key); - return !needsReadRetry(remoteOutput, retryConditions); - }, - ); - } else { - remoteOutput = yield* step.run("read:remote", () => - params.resource.read!(params.resource.key), - ); + if (remote.status === "absent") { + await emitLifecycleEvent(params, "read", "skip", { + reason: "resource-absent", + }); + return remote; } const mergedOutput = { ...resourceParams, - ...remoteOutput, + ...remote.output, }; await emitLifecycleEvent(params, "read", "success"); - return mergedOutput; + return { status: "found", output: mergedOutput }; } catch (err) { await emitLifecycleEvent(params, "read", "error", getErrorDetails(err)); throw err; diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index 3def119..f27dc40 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -1,8 +1,4 @@ -import type { - BaseResource, - ErrorMatcher, - ResourceType, -} from "@notation/resource"; +import type { BaseResource, ResourceType } from "@notation/resource"; import type { State } from "@notation/state"; export type OperationName = "create" | "read" | "update" | "delete"; @@ -63,7 +59,9 @@ export type CreateResourceParams = ResourceOperationBaseParams & { expectedRev: number; }; -export type ReadResourceParams = ResourceOperationBaseParams; +export type ReadResourceParams = ResourceOperationBaseParams & { + retryAbsent?: boolean; +}; export type UpdateResourceParams = ResourceOperationBaseParams & { patch: Record; @@ -84,28 +82,6 @@ export const DEFAULT_READ_POLL_OPTIONS: PollOptions = { retryInterval: 1_000, }; -export function matchError( - err: unknown, - matchers: ErrorMatcher[] | undefined, -): ErrorMatcher | undefined { - if (!matchers || matchers.length === 0) return undefined; - - const name = - typeof err === "object" && err && "name" in err - ? String((err as { name?: unknown }).name) - : undefined; - const message = - typeof err === "object" && err && "message" in err - ? String((err as { message?: unknown }).message) - : undefined; - - return matchers.find((matcher) => { - if (matcher.name !== name) return false; - if (matcher.message && matcher.message !== message) return false; - return true; - }); -} - export function getErrorDetails(err: unknown): { errorName: string; errorMessage: string; diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index 55dcba1..95f85a5 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -1,11 +1,11 @@ import { RetryableError, createWorkflow } from "yieldstar"; +import { RetryableResourceError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type StepRunner, type UpdateResourceParams, emitLifecycleEvent, getErrorDetails, - matchError, } from "./operation.types"; import { readResourceOperation } from "./operation.read"; @@ -42,9 +42,8 @@ export async function* updateResourceOperation( params.resource.toState(params.resource.output), ); } catch (err) { - const matcher = matchError(err, params.resource.retryLaterOnError); - if (matcher) { - throw new RetryableError(matcher.reason, { + if (err instanceof RetryableResourceError) { + throw new RetryableError(err.message, { ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), }); } @@ -62,11 +61,17 @@ export async function* updateResourceOperation( state: params.state, emit: params.emit, readPollOptions: params.readPollOptions, + retryAbsent: true, }); + if (readResult.status !== "found") { + throw new Error( + "Post-update read completed without finding the resource", + ); + } params.resource.setOutput({ ...params.resource.output, - ...readResult, + ...readResult.output, }); yield* step.run("update:persist-state", async () => { diff --git a/packages/reconciler/src/plan.ts b/packages/reconciler/src/plan.ts index 87fb621..c9ec40c 100644 --- a/packages/reconciler/src/plan.ts +++ b/packages/reconciler/src/plan.ts @@ -115,22 +115,19 @@ export function decideAction(opts: { export async function resolvePlanParams( resource: BaseResource, ): Promise> { - let resolved: Record | undefined; - try { - resolved = (await resource.getParams()) as Record; - } catch { - resolved = undefined; - } - - const params: Record = {}; - - if (resolved) { + const hasUnresolvedDependency = Object.values(resource.dependencies).some( + (dependency) => dependency && dependency.output == null, + ); + if (!hasUnresolvedDependency) { + const resolved = (await resource.getParams()) as Record; + const params: Record = {}; for (const [key, value] of Object.entries(resolved)) { params[key] = value === undefined ? UNKNOWN_AFTER_APPLY : value; } return params; } + const params: Record = {}; const config = resource.config as Record; for (const [key, item] of Object.entries(resource.schema)) { if (item.propertyType === "computed") continue; diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index 2db4628..ffcfd60 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -15,7 +15,6 @@ import { import { createResourceOperation, deleteResourceOperation, - matchError, readResourceOperation, type OperationLifecycleEvent, type PollOptions, @@ -446,21 +445,17 @@ export class Reconciler { } async #readForDrift(resource: BaseResource): Promise { - try { - const output = await runOperation( - readResourceOperation(this.#stepRunner, { - resource, - state: this.#state, - emit: this.#emit, - readPollOptions: this.#readPollOptions, - }), - ); - return { status: "found", output }; - } catch (err) { - const matcher = matchError(err, resource.notFoundOnError); - if (!matcher) throw err; - return { status: "not-found" }; - } + const result = await runOperation( + readResourceOperation(this.#stepRunner, { + resource, + state: this.#state, + emit: this.#emit, + readPollOptions: this.#readPollOptions, + }), + ); + return result.status === "found" + ? { status: "found", output: result.output } + : { status: "not-found" }; } async #deleteOrphans( diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index 8709086..ac03552 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { RetryableError } from "yieldstar"; -import { resource } from "@notation/resource"; +import { resource, RetryableResourceError } from "@notation/resource"; import { createResourceOperation, deleteResourceOperation, @@ -38,8 +38,7 @@ function createStepRunnerDouble(): StepRunner { ): AsyncGenerator { const opts = (typeof arg1 === "string" ? arg2 : arg1) as PollOptions; const predicate = (typeof arg1 === "string" ? arg3 : arg2) as - | (() => boolean | Promise) - | undefined; + (() => boolean | Promise) | undefined; if (!predicate) { throw new Error("Missing poll predicate"); @@ -57,7 +56,11 @@ function createStepRunnerDouble(): StepRunner { }); }); - const delay = vi.fn(async function* (): AsyncGenerator { + const delay = vi.fn(async function* (): AsyncGenerator< + unknown, + void, + unknown + > { return; }); @@ -90,9 +93,7 @@ describe("operation workflows", () => { const createMock = vi.fn(async () => { createAttempts += 1; if (createAttempts === 1) { - const err = new Error("eventual consistency"); - err.name = "RetryCreate"; - throw err; + throw new RetryableResourceError("retry create"); } return { remoteId: "abc" }; }); @@ -101,9 +102,11 @@ describe("operation workflows", () => { .defineSchema({}) .defineOperations({ create: createMock, - read: async () => ({ remoteId: "abc", status: "ready" }), + read: async () => ({ + status: "found", + output: { remoteId: "abc", status: "ready" }, + }), delete: async () => undefined, - retryLaterOnError: [{ name: "RetryCreate", reason: "retry create" }], }); const testResource = new TestResource({ id: "test-create" }); @@ -123,12 +126,9 @@ describe("operation workflows", () => { expect(state.update).toHaveBeenCalledOnce(); expect(createMock).toHaveBeenCalledWith(await testResource.getParams()); expect(testResource.output).toEqual({ remoteId: "abc", status: "ready" }); - expect(events.map((event) => `${event.operation}:${event.status}`)).toEqual([ - "create:start", - "read:start", - "read:success", - "create:success", - ]); + expect(events.map((event) => `${event.operation}:${event.status}`)).toEqual( + ["create:start", "read:start", "read:success", "create:success"], + ); expect(events[0]).toMatchObject({ resourceId: "test-create", resourceType: TestResource.type, @@ -136,7 +136,7 @@ describe("operation workflows", () => { }); }); - it("read uses durable polling semantics for retryReadOnCondition", async () => { + it("read retries while the resource reports a pending outcome", async () => { const step = createStepRunnerDouble(); const state = { get: vi.fn(async () => undefined), @@ -152,18 +152,17 @@ describe("operation workflows", () => { read: async () => { readAttempts += 1; if (readAttempts < 3) { - return { status: "pending" }; + return { + status: "pending", + reason: "resource is not ready", + } as const; } - return { status: "ready" }; + return { + status: "found", + output: { status: "ready" }, + } as const; }, delete: async () => undefined, - retryReadOnCondition: [ - { - key: "status", - value: "ready", - reason: "resource is not ready", - }, - ], }); const testResource = new TestResource({ id: "test-read" }); @@ -176,11 +175,48 @@ describe("operation workflows", () => { ); expect(readAttempts).toBe(3); - expect((step.poll as any).mock.calls.length).toBe(1); - expect(result).toEqual({ status: "ready" }); + expect(result).toEqual({ + status: "found", + output: { status: "ready" }, + }); }); - it("delete treats only resource.notFoundOnError matchers as skip", async () => { + it("retries an absent read after creation until the resource is visible", async () => { + const step = createStepRunnerDouble(); + const state = { + get: vi.fn(async () => undefined), + update: vi.fn(async () => undefined), + delete: vi.fn(async () => undefined), + }; + let readAttempts = 0; + const TestResource = resource({ type: "test/service/eventually-visible" }) + .defineSchema({}) + .defineOperations({ + create: async () => ({}), + read: async () => { + readAttempts += 1; + if (readAttempts === 1) return { status: "absent" } as const; + return { + status: "found", + output: { remoteId: "visible" }, + } as const; + }, + delete: async () => undefined, + }); + + await runOperation( + createResourceOperation(step, { + resource: new TestResource({ id: "eventually-visible" }), + state, + expectedRev: 0, + }), + ); + + expect(readAttempts).toBe(2); + expect(state.update).toHaveBeenCalledOnce(); + }); + + it("delete treats an already-absent remote as success through its idempotent resource contract", async () => { const step = createStepRunnerDouble(); const events: OperationLifecycleEvent[] = []; const state = { @@ -193,17 +229,7 @@ describe("operation workflows", () => { .defineSchema({}) .defineOperations({ create: async () => ({}), - delete: async () => { - const err = new Error("gone"); - err.name = "RemoteMissing"; - throw err; - }, - notFoundOnError: [ - { - name: "RemoteMissing", - reason: "already deleted remotely", - }, - ], + delete: async () => undefined, }); const testResource = new TestResource({ id: "test-delete" }); @@ -220,14 +246,10 @@ describe("operation workflows", () => { ); expect(state.delete).toHaveBeenCalledWith("test-delete", 1); - expect(events.map((event) => event.status)).toEqual([ - "start", - "skip", - "success", - ]); + expect(events.map((event) => event.status)).toEqual(["start", "success"]); }); - it("delete rethrows when error does not match notFoundOnError", async () => { + it("delete rethrows an unclassified resource error", async () => { const step = createStepRunnerDouble(); const state = { get: vi.fn(async () => undefined), @@ -244,12 +266,6 @@ describe("operation workflows", () => { err.name = "DifferentError"; throw err; }, - notFoundOnError: [ - { - name: "RemoteMissing", - reason: "already deleted remotely", - }, - ], }); const testResource = new TestResource({ id: "test-delete-miss" }); @@ -262,7 +278,10 @@ describe("operation workflows", () => { expectedRev: 1, }), ), - ).rejects.toMatchObject({ name: "DifferentError", message: "still exists" }); + ).rejects.toMatchObject({ + name: "DifferentError", + message: "still exists", + }); expect(state.delete).not.toHaveBeenCalled(); }); diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index 7220a65..ed10726 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { resource, type ErrorMatcher } from "@notation/resource"; +import { resource, type ResourceReadResult } from "@notation/resource"; import { LeaseConflict, MemoryStateBackend, @@ -59,7 +59,9 @@ function createTestResourceClass(opts: { create?: ( params: Record, ) => Promise | void>; - read?: (key: Record) => Promise>; + read?: ( + key: Record, + ) => Promise>>; update?: ( key: Record, patch: Record, @@ -70,7 +72,6 @@ function createTestResourceClass(opts: { key: Record, state: Record, ) => Promise; - notFoundOnError?: ErrorMatcher[]; }) { return resource({ type: opts.type }) .defineSchema({ @@ -85,11 +86,12 @@ function createTestResourceClass(opts: { read: opts.read, update: opts.update, delete: opts.delete ?? (async () => undefined), - notFoundOnError: opts.notFoundOnError, }); } const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +const found = (output: Record) => + ({ status: "found", output }) as const; describe("reconciler deploy", () => { it("chooses create vs update from desired params vs state", async () => { @@ -99,12 +101,12 @@ describe("reconciler deploy", () => { const CreateResource = createTestResourceClass({ type: "test/service/create-choice", create: createSpy, - read: async () => ({ name: "new" }), + read: async () => found({ name: "new" }), }); const UpdateResource = createTestResourceClass({ type: "test/service/update-choice", update: updateSpy, - read: async () => ({ name: "new" }), + read: async () => found({ name: "new" }), }); const state = createMemoryState({ @@ -149,7 +151,7 @@ describe("reconciler deploy", () => { const CreateResource = createTestResourceClass({ type: "test/service/first-create", create: async () => ({ name: "new" }), - read: async () => ({ name: "new" }), + read: async () => found({ name: "new" }), }); const state = createMemoryState(); const reconciler = new Reconciler({ state, driftDetection: false }); @@ -178,7 +180,7 @@ describe("reconciler deploy", () => { const CreateResource = createTestResourceClass({ type: "test/service/concurrent-create", create: createSpy, - read: async () => ({ name: "new" }), + read: async () => found({ name: "new" }), }); const state = new MemoryStateBackend(); const first = new Reconciler({ state, driftDetection: false }); @@ -202,7 +204,7 @@ describe("reconciler deploy", () => { it("reads remote state after an update conflict instead of repeating the update", async () => { let remoteName = "old"; - const readSpy = vi.fn(async () => ({ name: remoteName })); + const readSpy = vi.fn(async () => found({ name: remoteName })); const updateSpy = vi.fn(async (_key, _patch, params) => { remoteName = params.name as string; }); @@ -268,7 +270,7 @@ describe("reconciler deploy", () => { remoteName = params.name as string; return { name: remoteName }; }); - const readSpy = vi.fn(async () => ({ name: remoteName! })); + const readSpy = vi.fn(async () => found({ name: remoteName! })); const CreateResource = createTestResourceClass({ type: "test/service/create-conflict", create: createSpy, @@ -360,7 +362,7 @@ describe("reconciler deploy", () => { marks.aEnd = Date.now(); return { name: "a" }; }, - read: async () => ({ name: "a" }), + read: async () => found({ name: "a" }), }); const CResource = createTestResourceClass({ type: "test/service/c", @@ -370,7 +372,7 @@ describe("reconciler deploy", () => { marks.cEnd = Date.now(); return { name: "c" }; }, - read: async () => ({ name: "c" }), + read: async () => found({ name: "c" }), }); const BResource = createTestResourceClass({ type: "test/service/b", @@ -378,7 +380,7 @@ describe("reconciler deploy", () => { marks.bStart = Date.now(); return { name: "b" }; }, - read: async () => ({ name: "b" }), + read: async () => found({ name: "b" }), }); const state = createMemoryState(); @@ -402,7 +404,7 @@ describe("reconciler deploy", () => { const events: Array> = []; const TestResource = createTestResourceClass({ type: "test/service/drift", - read: async () => ({ name: "drifted" }), + read: async () => found({ name: "drifted" }), update: updateSpy, }); @@ -484,7 +486,7 @@ describe("reconciler deploy", () => { const CreateResource = createTestResourceClass({ type: "test/service/dry-run-create", create: createSpy, - read: async () => ({ name: "new" }), + read: async () => found({ name: "new" }), }); const OrphanResource = createTestResourceClass({ type: "test/service/dry-run-orphan", @@ -541,18 +543,13 @@ describe("reconciler destroy + refresh", () => { remoteExists = false; }); const readSpy = vi.fn(async () => { - if (!remoteExists) { - const error = new Error("gone"); - error.name = "RemoteMissing"; - throw error; - } - return { name: "doomed" }; + if (!remoteExists) return { status: "absent" } as const; + return found({ name: "doomed" }); }); const DestroyResource = createTestResourceClass({ type: "test/service/destroy-retry", read: readSpy, delete: deleteSpy, - notFoundOnError: [{ name: "RemoteMissing", reason: "deleted" }], }); const state = createMemoryState({ doomed: { diff --git a/packages/reconciler/test/reconciler.plan.test.ts b/packages/reconciler/test/reconciler.plan.test.ts index 88b8c91..b3c1fd8 100644 --- a/packages/reconciler/test/reconciler.plan.test.ts +++ b/packages/reconciler/test/reconciler.plan.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { resource, type BaseResource } from "@notation/resource"; +import { + resource, + type BaseResource, + type ResourceReadResult, +} from "@notation/resource"; import type { StateNode } from "@notation/state"; import { Reconciler, UNKNOWN_AFTER_APPLY } from "../src"; @@ -37,7 +41,9 @@ function createTestResourceClass(opts: { create?: ( params: Record, ) => Promise | void>; - read?: (key: Record) => Promise>; + read?: ( + key: Record, + ) => Promise>>; update?: ( key: Record, patch: Record, @@ -48,7 +54,6 @@ function createTestResourceClass(opts: { key: Record, state: Record, ) => Promise; - notFoundOnError?: { name: string; reason: string }[]; }) { return resource({ type: opts.type }) .defineSchema({ @@ -68,7 +73,6 @@ function createTestResourceClass(opts: { read: opts.read, update: opts.update, delete: opts.delete ?? (async () => undefined), - notFoundOnError: opts.notFoundOnError, }); } @@ -168,7 +172,10 @@ describe("reconciler plan", () => { }); it("plans drift-update from live read output when drift detection is on", async () => { - const readSpy = vi.fn(async () => ({ name: "drifted" })); + const readSpy = vi.fn(async () => ({ + status: "found" as const, + output: { name: "drifted" }, + })); const TestResource = createTestResourceClass({ type: "test/service/plan-drift-update", read: readSpy, @@ -203,14 +210,7 @@ describe("reconciler plan", () => { it("plans drift-recreate when the remote resource is gone", async () => { const TestResource = createTestResourceClass({ type: "test/service/plan-drift-recreate", - read: async () => { - const err = new Error("gone"); - err.name = "NotFoundException"; - throw err; - }, - notFoundOnError: [ - { name: "NotFoundException", reason: "deleted remotely" }, - ], + read: async () => ({ status: "absent" }), }); const state = createMemoryState({ @@ -233,7 +233,10 @@ describe("reconciler plan", () => { }); it("skips remote reads when drift detection is off", async () => { - const readSpy = vi.fn(async () => ({ name: "drifted" })); + const readSpy = vi.fn(async () => ({ + status: "found" as const, + output: { name: "drifted" }, + })); const TestResource = createTestResourceClass({ type: "test/service/plan-no-read", read: readSpy, @@ -332,6 +335,35 @@ describe("reconciler plan", () => { }); }); + it("does not disguise parameter derivation failures as unknown values", async () => { + const TestResource = resource({ + type: "test/service/plan-derive-failure", + }) + .defineSchema({ + name: { + presence: "required", + propertyType: "param", + valueType: "string" as any, + }, + }) + .defineOperations({ + create: async () => ({}), + delete: async () => undefined, + deriveParams: () => { + throw new Error("invalid derived configuration"); + }, + }); + + const reconciler = new Reconciler({ + state: createMemoryState(), + driftDetection: false, + }); + + await expect( + reconciler.plan([new TestResource({ id: "broken" })]), + ).rejects.toThrow("invalid derived configuration"); + }); + it("produces a JSON-round-trippable plan", async () => { const CreateResource = createTestResourceClass({ type: "test/service/plan-json-create", @@ -375,7 +407,10 @@ describe("reconciler plan", () => { create: createSpy, update: updateSpy, delete: deleteSpy, - read: async () => ({ name: "drifted" }), + read: async () => ({ + status: "found", + output: { name: "drifted" }, + }), }); const state = createMemoryState({ diff --git a/packages/resource/src/resource.ts b/packages/resource/src/resource.ts index b2ed61d..bf1271e 100644 --- a/packages/resource/src/resource.ts +++ b/packages/resource/src/resource.ts @@ -22,21 +22,19 @@ export type { Schema, SchemaItem, DefineResourceApiSchema }; export type ResourceType = `${string}/${string}/${string}`; -export type ErrorMatcher = { - name: string; - message?: string; - reason: string; -}; +export type ResourceReadResult = + | { status: "found"; output: T } + | { status: "absent" } + | { status: "pending"; reason: string }; -export type ResultCondition = { - key: K; - reason: string; - value?: T[K]; -}; +export class RetryableResourceError extends Error { + readonly code = "RESOURCE_RETRYABLE"; -export type ResultConditions = { - [K in keyof T]?: ResultCondition; -}[keyof T][]; + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = "RetryableResourceError"; + } +} export type ResourceOpts = OptionalIfAllPropertiesOptional<"config", C> & OptionalIfAllPropertiesOptional<"dependencies", D> & { id: string }; @@ -77,17 +75,9 @@ export interface BaseResource { groupType: string; readonly output: {}; readonly dependencies: Record; - readonly retryReadOnCondition?: ({ - key: any; - value?: any; - reason: string; - } | void)[]; - readonly failOnError?: (ErrorMatcher & { reason: string })[]; - readonly notFoundOnError?: ErrorMatcher[]; - readonly retryLaterOnError?: ErrorMatcher[]; readonly key: {}; create: (params: any) => Promise<{} | void>; - read?: (key: any) => Promise>; + read?: (key: any) => Promise>>; update?: (key: any, patch: any, params: any, state: any) => Promise; delete: (key: any, state: any) => Promise; getParams(): Promise<{}>; @@ -115,7 +105,9 @@ export abstract class Resource< abstract type: ResourceType; abstract schema: Schema; abstract create: (params: T["params"]) => Promise; - abstract read?: (key: T["compoundKey"]) => Promise; + abstract read?: ( + key: T["compoundKey"], + ) => Promise>; abstract update?: ( key: T["compoundKey"], patch: T["params"], @@ -123,10 +115,6 @@ export abstract class Resource< state: T["state"], ) => Promise; abstract delete: (key: T["compoundKey"], state: T["state"]) => Promise; - abstract retryReadOnCondition?: ResultConditions; - abstract failOnError?: (ErrorMatcher & { reason: string })[]; - abstract notFoundOnError?: ErrorMatcher[]; - abstract retryLaterOnError?: ErrorMatcher[]; abstract deriveParams(opts: { id: string; config: C; @@ -199,7 +187,7 @@ export type ResourceOperationsOptions< IntrinsicParams extends Partial, > = { create: (params: T["params"]) => Promise; - read?: (key: T["compoundKey"]) => Promise; + read?: (key: T["compoundKey"]) => Promise>; update?: ( key: T["compoundKey"], patch: T["params"], @@ -207,10 +195,6 @@ export type ResourceOperationsOptions< state: T["state"], ) => Promise; delete: (key: T["compoundKey"], state: T["state"]) => Promise; - retryReadOnCondition?: ResultConditions; - failOnError?: (ErrorMatcher & { reason: string })[]; - notFoundOnError?: ErrorMatcher[]; - retryLaterOnError?: ErrorMatcher[]; deriveParams?: (opts: { config: Partial; }) => IntrinsicParams | Promise; @@ -315,10 +299,6 @@ export function defineResource( read = opts.read ? opts.read : undefined; update = opts.update ? opts.update : undefined; delete = opts.delete; - retryReadOnCondition = opts.retryReadOnCondition; - failOnError = opts.failOnError; - notFoundOnError = opts.notFoundOnError; - retryLaterOnError = opts.retryLaterOnError; async deriveParams() { if (!opts.deriveParams) return {}; diff --git a/packages/resource/test/resource.doubles.ts b/packages/resource/test/resource.doubles.ts index 1293849..a689c59 100644 --- a/packages/resource/test/resource.doubles.ts +++ b/packages/resource/test/resource.doubles.ts @@ -65,7 +65,14 @@ export const testOperations = { async delete() {}, async update() {}, async read() { - return { primaryKey: "", optionalSecondaryKey: "", requiredParam: "" }; + return { + status: "found", + output: { + primaryKey: "", + optionalSecondaryKey: "", + requiredParam: "", + }, + } as const; }, deriveParams() { return { intrinsicParam: true }; diff --git a/packages/std.iac/src/resources/fs/file.ts b/packages/std.iac/src/resources/fs/file.ts index 52509a0..ff1b9f9 100644 --- a/packages/std.iac/src/resources/fs/file.ts +++ b/packages/std.iac/src/resources/fs/file.ts @@ -36,8 +36,16 @@ export const File = fileSchema.defineOperations({ return { sourceSha256 }; }, read: async (config) => { - const file = await fs.readFile(config.filePath); - return { ...config, file }; + try { + const file = await fs.readFile(config.filePath); + return { + status: "found", + output: { ...config, file }, + } as const; + } catch (error) { + if (isFileMissing(error)) return { status: "absent" } as const; + throw error; + } }, create: async () => {}, update: async () => {}, @@ -45,3 +53,12 @@ export const File = fileSchema.defineOperations({ }); export type FileInstance = InstanceType; + +function isFileMissing(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ); +} diff --git a/packages/std.iac/src/resources/fs/zip.ts b/packages/std.iac/src/resources/fs/zip.ts index f8d5585..3a5a4a1 100644 --- a/packages/std.iac/src/resources/fs/zip.ts +++ b/packages/std.iac/src/resources/fs/zip.ts @@ -53,12 +53,13 @@ export const Zip = zipSchema.defineOperations({ read: async (params) => { try { const file = await fs.readFile(params.filePath); - return { ...params, file }; - } catch (error: any) { - if (error.code !== "ENOENT") throw error; - await zip.package(params.sourceFilePath, params.filePath); - const file = await fs.readFile(params.filePath); - return { ...params, file }; + return { + status: "found", + output: { ...params, file }, + } as const; + } catch (error) { + if (isFileMissing(error)) return { status: "absent" } as const; + throw error; } }, create: async (params) => { @@ -69,8 +70,21 @@ export const Zip = zipSchema.defineOperations({ await zip.package(config.sourceFilePath, config.filePath); }, delete: async (config) => { - await fs.unlink(config.filePath); + try { + await fs.unlink(config.filePath); + } catch (error) { + if (!isFileMissing(error)) throw error; + } }, }); export type ZipFileInstance = InstanceType; + +function isFileMissing(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ); +} From 031b9d2f7d84ee1feb55271dbc4d710501367f12 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:44:49 +0100 Subject: [PATCH 2/6] Share error code recognition --- .changeset/resource-operation-outcomes.md | 2 ++ examples/reconciler/package.json | 3 ++- examples/reconciler/src/static-site.ts | 14 ++++-------- packages/state/package.json | 3 +++ packages/state/src/state.ts | 26 +++++------------------ packages/std.iac/package.json | 1 + packages/std.iac/src/resources/fs/file.ts | 14 ++++-------- packages/std.iac/src/resources/fs/zip.ts | 16 +++++--------- packages/utils/package.json | 18 ++++++++++++++++ packages/utils/src/index.ts | 11 ++++++++++ packages/utils/test/index.test.ts | 13 ++++++++++++ packages/utils/tsconfig.json | 7 ++++++ packages/utils/tsup.config.ts | 7 ++++++ pnpm-lock.yaml | 16 ++++++++++++++ 14 files changed, 98 insertions(+), 53 deletions(-) create mode 100644 packages/utils/package.json create mode 100644 packages/utils/src/index.ts create mode 100644 packages/utils/test/index.test.ts create mode 100644 packages/utils/tsconfig.json create mode 100644 packages/utils/tsup.config.ts diff --git a/.changeset/resource-operation-outcomes.md b/.changeset/resource-operation-outcomes.md index 580b357..43d6b2a 100644 --- a/.changeset/resource-operation-outcomes.md +++ b/.changeset/resource-operation-outcomes.md @@ -3,7 +3,9 @@ "@notation/core": minor "@notation/reconciler": minor "@notation/resource": minor +"@notation/state": minor "@notation/std.iac": minor +"@notation/utils": minor --- Replace error matcher declarations and read retry conditions with explicit resource read outcomes and retryable resource errors. diff --git a/examples/reconciler/package.json b/examples/reconciler/package.json index 754b89a..55cbaa1 100644 --- a/examples/reconciler/package.json +++ b/examples/reconciler/package.json @@ -11,7 +11,8 @@ "dependencies": { "@notation/reconciler": "workspace:*", "@notation/resource": "workspace:*", - "@notation/state-sqlite": "workspace:*" + "@notation/state-sqlite": "workspace:*", + "@notation/utils": "workspace:*" }, "devDependencies": { "@types/node": "^22.13.4", diff --git a/examples/reconciler/src/static-site.ts b/examples/reconciler/src/static-site.ts index 5b30db0..64a44ae 100644 --- a/examples/reconciler/src/static-site.ts +++ b/examples/reconciler/src/static-site.ts @@ -1,6 +1,7 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; type StaticSiteApi = { Key: { siteDirectory: string }; @@ -36,7 +37,9 @@ export const StaticSite = staticSite ); return { status: "found", output: { html } } as const; } catch (error) { - if (isFileMissing(error)) return { status: "absent" } as const; + if (isErrorWithCode(error, "ENOENT")) { + return { status: "absent" } as const; + } throw error; } }, @@ -47,12 +50,3 @@ export const StaticSite = staticSite await rm(siteDirectory, { recursive: true, force: true }); }, }); - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/state/package.json b/packages/state/package.json index 2c81b57..8a49f6f 100644 --- a/packages/state/package.json +++ b/packages/state/package.json @@ -11,6 +11,9 @@ "build": "tsup --clean", "dev": "tsup --watch" }, + "dependencies": { + "@notation/utils": "workspace:*" + }, "devDependencies": { "@types/node": "^22.13.4" } diff --git a/packages/state/src/state.ts b/packages/state/src/state.ts index 003faa1..d0c10c1 100644 --- a/packages/state/src/state.ts +++ b/packages/state/src/state.ts @@ -9,6 +9,7 @@ import { } from "node:fs/promises"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; +import { isErrorWithCode } from "@notation/utils"; import { LeaseConflict, RevConflict } from "./conflicts"; export type StateNode = { @@ -222,7 +223,7 @@ export class FileStateBackend implements StateBackend { }); break; } catch (error) { - if (!isFileExistsError(error)) throw error; + if (!isErrorWithCode(error, "EEXIST")) throw error; const current = await readFileLease(leaseFilePath); if (!current || current.expiresAtMs <= Date.now()) { await unlink(leaseFilePath).catch(() => undefined); @@ -271,7 +272,7 @@ export class FileStateBackend implements StateBackend { const file = await readFile(this.stateFilePath, "utf8"); return JSON.parse(file) as Record; } catch (error) { - if (isFileMissingError(error)) { + if (isErrorWithCode(error, "ENOENT")) { return {}; } @@ -298,7 +299,7 @@ export class FileStateBackend implements StateBackend { ); break; } catch (error) { - if (!isFileExistsError(error)) throw error; + if (!isErrorWithCode(error, "EEXIST")) throw error; const lockStat = await stat(lockFilePath).catch(() => undefined); if (lockStat && Date.now() - lockStat.mtimeMs > FILE_LOCK_STALE_MS) { await unlink(lockFilePath).catch(() => undefined); @@ -367,29 +368,12 @@ async function readFileLease( try { return JSON.parse(await readFile(filePath, "utf8")) as FileLeaseRecord; } catch (error) { - if (isFileMissingError(error) || error instanceof SyntaxError) + if (isErrorWithCode(error, "ENOENT") || error instanceof SyntaxError) return undefined; throw error; } } -function isFileMissingError(error: unknown): boolean { - return isErrorWithCode(error, "ENOENT"); -} - -function isFileExistsError(error: unknown): boolean { - return isErrorWithCode(error, "EEXIST"); -} - -function isErrorWithCode(error: unknown, code: string): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === code - ); -} - function cloneAsPersistedState( state: Record, ): Record { diff --git a/packages/std.iac/package.json b/packages/std.iac/package.json index 63cfaa2..90d9789 100644 --- a/packages/std.iac/package.json +++ b/packages/std.iac/package.json @@ -15,6 +15,7 @@ "dependencies": { "@notation/core": "workspace:*", "@notation/resource": "workspace:*", + "@notation/utils": "workspace:*", "fflate": "0.8.2" }, "devDependencies": { diff --git a/packages/std.iac/src/resources/fs/file.ts b/packages/std.iac/src/resources/fs/file.ts index ff1b9f9..2bfd155 100644 --- a/packages/std.iac/src/resources/fs/file.ts +++ b/packages/std.iac/src/resources/fs/file.ts @@ -1,4 +1,5 @@ import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; import { getSourceSha256 } from "src/utils/hash"; import * as fs from "node:fs/promises"; @@ -43,7 +44,9 @@ export const File = fileSchema.defineOperations({ output: { ...config, file }, } as const; } catch (error) { - if (isFileMissing(error)) return { status: "absent" } as const; + if (isErrorWithCode(error, "ENOENT")) { + return { status: "absent" } as const; + } throw error; } }, @@ -53,12 +56,3 @@ export const File = fileSchema.defineOperations({ }); export type FileInstance = InstanceType; - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/std.iac/src/resources/fs/zip.ts b/packages/std.iac/src/resources/fs/zip.ts index 3a5a4a1..57d1caa 100644 --- a/packages/std.iac/src/resources/fs/zip.ts +++ b/packages/std.iac/src/resources/fs/zip.ts @@ -1,4 +1,5 @@ import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; import * as fs from "node:fs/promises"; import { zip } from "src/utils/zip"; import { getSourceSha256 } from "src/utils/hash"; @@ -58,7 +59,9 @@ export const Zip = zipSchema.defineOperations({ output: { ...params, file }, } as const; } catch (error) { - if (isFileMissing(error)) return { status: "absent" } as const; + if (isErrorWithCode(error, "ENOENT")) { + return { status: "absent" } as const; + } throw error; } }, @@ -73,18 +76,9 @@ export const Zip = zipSchema.defineOperations({ try { await fs.unlink(config.filePath); } catch (error) { - if (!isFileMissing(error)) throw error; + if (!isErrorWithCode(error, "ENOENT")) throw error; } }, }); export type ZipFileInstance = InstanceType; - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/utils/package.json b/packages/utils/package.json new file mode 100644 index 0000000..ef2ec23 --- /dev/null +++ b/packages/utils/package.json @@ -0,0 +1,18 @@ +{ + "type": "module", + "name": "@notation/utils", + "version": "0.12.0", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "files": [ + "dist" + ], + "scripts": { + "build": "tsup --clean", + "dev": "tsup --watch", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "@types/node": "^22.13.4" + } +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts new file mode 100644 index 0000000..fc8b93c --- /dev/null +++ b/packages/utils/src/index.ts @@ -0,0 +1,11 @@ +export function isErrorWithCode( + error: unknown, + code: string, +): error is { code: string } { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === code + ); +} diff --git a/packages/utils/test/index.test.ts b/packages/utils/test/index.test.ts new file mode 100644 index 0000000..99a969a --- /dev/null +++ b/packages/utils/test/index.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { isErrorWithCode } from "../src"; + +describe("isErrorWithCode", () => { + it("recognises an object with the requested code", () => { + expect(isErrorWithCode({ code: "ENOENT" }, "ENOENT")).toBe(true); + }); + + it("rejects other codes and non-object values", () => { + expect(isErrorWithCode({ code: "EEXIST" }, "ENOENT")).toBe(false); + expect(isErrorWithCode("ENOENT", "ENOENT")).toBe(false); + }); +}); diff --git a/packages/utils/tsconfig.json b/packages/utils/tsconfig.json new file mode 100644 index 0000000..4423630 --- /dev/null +++ b/packages/utils/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "tsconfig/base.json", + "compilerOptions": { + "baseUrl": ".", + "types": ["node"] + } +} diff --git a/packages/utils/tsup.config.ts b/packages/utils/tsup.config.ts new file mode 100644 index 0000000..f0ac238 --- /dev/null +++ b/packages/utils/tsup.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from "tsup"; + +export default defineConfig({ + entry: ["src/index.ts"], + dts: true, + format: ["esm"], +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0b23b2f..82d8d14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -126,6 +126,9 @@ importers: '@notation/state-sqlite': specifier: workspace:* version: link:../../packages/state-sqlite + '@notation/utils': + specifier: workspace:* + version: link:../../packages/utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -352,6 +355,10 @@ importers: packages/resource: {} packages/state: + dependencies: + '@notation/utils': + specifier: workspace:* + version: link:../utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -375,6 +382,9 @@ importers: '@notation/resource': specifier: workspace:* version: link:../resource + '@notation/utils': + specifier: workspace:* + version: link:../utils fflate: specifier: 0.8.2 version: 0.8.2 @@ -385,6 +395,12 @@ importers: packages/tsconfig: {} + packages/utils: + devDependencies: + '@types/node': + specifier: ^22.13.4 + version: 22.13.4 + packages: '@aws-sdk/client-apigatewayv2@3.1080.0': From 9940bffc1c66a62601e3f960e0849065d120cd4c Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 15:37:54 +0100 Subject: [PATCH 3/6] Return the read object or undefined A resource read now returns the remote object, or undefined when the provider says it does not exist. Provider not-found exceptions are still translated at the provider boundary; they just become undefined there. Known temporary conditions - a Lambda that is still deploying, an IAM role that has not propagated - throw ResourceNotReadyError, recognised by its declared _tag rather than by class identity or message text. The reconciler decides what that means: deploy, read and mutation workflows retry it, while planning reports an indeterminate decision carrying the message. Everything else propagates. Drops the shared error-code matcher in favour of local checks at each provider boundary. --- .changeset/resource-operation-outcomes.md | 5 +- docs/internals/resource.md | 8 ++- examples/reconciler/package.json | 3 +- examples/reconciler/src/static-site.ts | 16 +++-- .../aws.iac/src/resources/api-gateway/api.ts | 7 +-- .../aws.iac/src/resources/api-gateway/auth.ts | 4 +- .../api-gateway/lambda-integration.ts | 4 +- .../src/resources/api-gateway/route.ts | 7 +-- .../src/resources/api-gateway/stage.ts | 4 +- .../src/resources/event-bridge/rule.ts | 11 ++-- .../src/resources/lambda/lambda-role.ts | 4 +- .../aws.iac/src/resources/lambda/lambda.ts | 39 +++++------- packages/cli/src/plan.ts | 2 + packages/core/src/orchestrator/resource.ts | 3 +- .../test/orchestrator/resource.doubles.ts | 9 +-- .../test/provisioner/operation.create.test.ts | 5 +- .../src/operations/operation.create.ts | 8 +-- .../src/operations/operation.delete.ts | 4 +- .../src/operations/operation.read.ts | 53 ++++++++-------- .../src/operations/operation.types.ts | 2 + .../src/operations/operation.update.ts | 8 +-- packages/reconciler/src/plan.ts | 19 +++++- packages/reconciler/src/reconciler.ts | 60 ++++++++++++++----- .../test/operation.workflows.test.ts | 33 +++------- .../reconciler/test/reconciler.deploy.test.ts | 9 ++- .../reconciler/test/reconciler.plan.test.ts | 47 ++++++++++----- packages/resource/src/resource.ts | 36 +++++++---- packages/resource/test/resource.doubles.ts | 9 +-- packages/resource/test/resource.test.ts | 24 +++++++- packages/state/package.json | 3 - packages/state/src/state.ts | 26 ++++++-- packages/std.iac/package.json | 1 - packages/std.iac/src/resources/fs/file.ts | 19 +++--- packages/std.iac/src/resources/fs/zip.ts | 21 ++++--- packages/utils/package.json | 18 ------ packages/utils/src/index.ts | 11 ---- packages/utils/test/index.test.ts | 13 ---- packages/utils/tsconfig.json | 7 --- packages/utils/tsup.config.ts | 7 --- pnpm-lock.yaml | 16 ----- 40 files changed, 302 insertions(+), 283 deletions(-) delete mode 100644 packages/utils/package.json delete mode 100644 packages/utils/src/index.ts delete mode 100644 packages/utils/test/index.test.ts delete mode 100644 packages/utils/tsconfig.json delete mode 100644 packages/utils/tsup.config.ts diff --git a/.changeset/resource-operation-outcomes.md b/.changeset/resource-operation-outcomes.md index 43d6b2a..b7ccedc 100644 --- a/.changeset/resource-operation-outcomes.md +++ b/.changeset/resource-operation-outcomes.md @@ -1,11 +1,10 @@ --- "@notation/aws.iac": minor +"@notation/cli": minor "@notation/core": minor "@notation/reconciler": minor "@notation/resource": minor -"@notation/state": minor "@notation/std.iac": minor -"@notation/utils": minor --- -Replace error matcher declarations and read retry conditions with explicit resource read outcomes and retryable resource errors. +A resource `read` now returns the remote object, or `undefined` when it does not exist. Providers translate their own not-found exceptions at the boundary. Known temporary conditions — a Lambda that is still deploying, an IAM role that has not propagated — throw the tagged `ResourceNotReadyError`, which the reconciler retries during deploys and reports as an `indeterminate` plan decision. diff --git a/docs/internals/resource.md b/docs/internals/resource.md index 8afd5f1..25c94b4 100644 --- a/docs/internals/resource.md +++ b/docs/internals/resource.md @@ -142,12 +142,16 @@ All schema items carry these fields: | Field | Required | Signature / Description | | -------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------- | | `create` | yes | `(params: Params) => Promise>` – create the resource, return its computed key. | -| `read` | no | `(key: CompoundKey) => Promise>>` – report the remote as `found`, `absent`, or temporarily `pending`. | +| `read` | no | `(key: CompoundKey) => Promise \| undefined>` – return the remote object, or `undefined` when it does not exist. | | `update` | no | `(key, patch, params, state) => Promise` – apply a partial update. | | `delete` | yes | `(key, state) => Promise` – ensure the resource is absent. Implementations must also succeed when the remote resource is already gone. | | `deriveParams` | no | Computes intrinsic derived params from config (not dependency-aware). | -Resource operations translate provider-specific responses at the provider boundary. A read returns `{ status: "found", output }`, `{ status: "absent" }`, or `{ status: "pending", reason }`. A mutation throws `RetryableResourceError` when the provider explicitly reports a transient condition; all other errors fail the operation. +Resource operations translate provider-specific responses at the provider boundary. A read returns the remote object when it is found, and `undefined` when the provider says it does not exist — the provider's own not-found exception is caught and turned into `undefined` there, not further up the stack. + +When the provider reports a known temporary condition — a Lambda that is still deploying, an IAM role that has not propagated — a read or a mutation throws `ResourceNotReadyError`. It is recognised by its declared `_tag` via `ResourceNotReadyError.is(error)`, never by provider name, message, or `instanceof`. Every other error propagates and fails the operation. + +The reconciler decides what a not-ready condition means. Deploy, read, and mutation workflows retry it; planning reports it as an `indeterminate` decision carrying the error's message, because it cannot diff against a resource that has not settled. ## Dependencies diff --git a/examples/reconciler/package.json b/examples/reconciler/package.json index 55cbaa1..754b89a 100644 --- a/examples/reconciler/package.json +++ b/examples/reconciler/package.json @@ -11,8 +11,7 @@ "dependencies": { "@notation/reconciler": "workspace:*", "@notation/resource": "workspace:*", - "@notation/state-sqlite": "workspace:*", - "@notation/utils": "workspace:*" + "@notation/state-sqlite": "workspace:*" }, "devDependencies": { "@types/node": "^22.13.4", diff --git a/examples/reconciler/src/static-site.ts b/examples/reconciler/src/static-site.ts index 64a44ae..6528a2b 100644 --- a/examples/reconciler/src/static-site.ts +++ b/examples/reconciler/src/static-site.ts @@ -1,7 +1,6 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { resource } from "@notation/resource"; -import { isErrorWithCode } from "@notation/utils"; type StaticSiteApi = { Key: { siteDirectory: string }; @@ -35,10 +34,10 @@ export const StaticSite = staticSite path.join(siteDirectory, "index.html"), "utf8", ); - return { status: "found", output: { html } } as const; + return { html }; } catch (error) { - if (isErrorWithCode(error, "ENOENT")) { - return { status: "absent" } as const; + if (isFileMissing(error)) { + return undefined; } throw error; } @@ -50,3 +49,12 @@ export const StaticSite = staticSite await rm(siteDirectory, { recursive: true, force: true }); }, }); + +function isFileMissing(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ); +} diff --git a/packages/aws.iac/src/resources/api-gateway/api.ts b/packages/aws.iac/src/resources/api-gateway/api.ts index b0a323e..492d27e 100644 --- a/packages/aws.iac/src/resources/api-gateway/api.ts +++ b/packages/aws.iac/src/resources/api-gateway/api.ts @@ -102,13 +102,10 @@ export const Api = apiSchema.defineOperations({ const result = await apiGatewayClient.send(command); // todo: check types or correct or if RouteKey is actually in result // if not, need to pass the original params to read - return { - status: "found", - output: { RouteKey: "", ...result }, - } as const; + return { RouteKey: "", ...result }; } catch (error) { if (error instanceof sdk.NotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/auth.ts b/packages/aws.iac/src/resources/api-gateway/auth.ts index a4debaa..2f58785 100644 --- a/packages/aws.iac/src/resources/api-gateway/auth.ts +++ b/packages/aws.iac/src/resources/api-gateway/auth.ts @@ -62,10 +62,10 @@ export const RouteAuth = apiSchema try { const command = new sdk.GetAuthorizerCommand(key); const output = await apiGatewayClient.send(command); - return { status: "found", output } as const; + return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts index e34bfa6..75d9367 100644 --- a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts +++ b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts @@ -121,10 +121,10 @@ export const LambdaIntegration = integrationSchema try { const command = new sdk.GetIntegrationCommand(key); const output = await apiGatewayClient.send(command); - return { status: "found", output } as const; + return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/route.ts b/packages/aws.iac/src/resources/api-gateway/route.ts index 55a6661..6252449 100644 --- a/packages/aws.iac/src/resources/api-gateway/route.ts +++ b/packages/aws.iac/src/resources/api-gateway/route.ts @@ -95,13 +95,10 @@ export const Route = routeSchema try { const command = new sdk.GetRouteCommand(key); const result = await apiGatewayClient.send(command); - return { - status: "found", - output: { ...key, ...result }, - } as const; + return { ...key, ...result }; } catch (error) { if (error instanceof sdk.NotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/stage.ts b/packages/aws.iac/src/resources/api-gateway/stage.ts index 65d3c1d..73f37d6 100644 --- a/packages/aws.iac/src/resources/api-gateway/stage.ts +++ b/packages/aws.iac/src/resources/api-gateway/stage.ts @@ -79,10 +79,10 @@ export const Stage = stageSchema try { const command = new sdk.GetStageCommand(key); const output = await apiGatewayClient.send(command); - return { status: "found", output } as const; + return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/event-bridge/rule.ts b/packages/aws.iac/src/resources/event-bridge/rule.ts index 107a385..d423567 100644 --- a/packages/aws.iac/src/resources/event-bridge/rule.ts +++ b/packages/aws.iac/src/resources/event-bridge/rule.ts @@ -74,15 +74,12 @@ export const EventBridgeRule = eventBridgeRuleSchema ]); return { - status: "found", - output: { - ...ruleDescriptionResult, - ...listRuleTargetsResult, - }, - } as const; + ...ruleDescriptionResult, + ...listRuleTargetsResult, + }; } catch (error) { if (error instanceof sdk.ResourceNotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/lambda/lambda-role.ts b/packages/aws.iac/src/resources/lambda/lambda-role.ts index 18843d1..309712d 100644 --- a/packages/aws.iac/src/resources/lambda/lambda-role.ts +++ b/packages/aws.iac/src/resources/lambda/lambda-role.ts @@ -72,10 +72,10 @@ export const LambdaIamRole = lambdaIamRoleSchema.defineOperations({ try { const command = new sdk.GetRoleCommand(key); const { Role } = await iamClient.send(command); - return { status: "found", output: Role! } as const; + return Role!; } catch (error) { if (error instanceof sdk.NoSuchEntityException) { - return { status: "absent" } as const; + return undefined; } throw error; } diff --git a/packages/aws.iac/src/resources/lambda/lambda.ts b/packages/aws.iac/src/resources/lambda/lambda.ts index 97e4582..19cafa8 100644 --- a/packages/aws.iac/src/resources/lambda/lambda.ts +++ b/packages/aws.iac/src/resources/lambda/lambda.ts @@ -1,4 +1,4 @@ -import { resource, RetryableResourceError, typed } from "@notation/resource"; +import { resource, ResourceNotReadyError, typed } from "@notation/resource"; import * as sdk from "@aws-sdk/client-lambda"; import { lambdaClient } from "src/utils/aws-clients"; import { AwsSchema } from "src/utils/types"; @@ -204,35 +204,28 @@ export const LambdaFunction = lambdaFunctionSchema await lambdaClient.send(command); if (Configuration?.State !== "Active") { - return { - status: "pending", - reason: "Waiting for Lambda to become active", - } as const; + throw new ResourceNotReadyError( + "Waiting for Lambda to become active", + ); } if (!Configuration.RevisionId) { - return { - status: "pending", - reason: "Waiting for Lambda to be deployed", - } as const; + throw new ResourceNotReadyError("Waiting for Lambda to be deployed"); } return { - status: "found", - output: { - ...Configuration, - Layers: Configuration.Layers?.map((layer) => layer.Arn), - ...Concurrency, - Code: { - S3Bucket: Code?.Location?.split("/")[0], - S3Key: Code?.Location?.split("/")[1], - S3ObjectVersion: Code?.Location?.split("/")[2], - ZipFile: undefined, - }, + ...Configuration, + Layers: Configuration.Layers?.map((layer) => layer.Arn), + ...Concurrency, + Code: { + S3Bucket: Code?.Location?.split("/")[0], + S3Key: Code?.Location?.split("/")[1], + S3ObjectVersion: Code?.Location?.split("/")[2], + ZipFile: undefined, }, - } as const; + }; } catch (error) { if (error instanceof sdk.ResourceNotFoundException) { - return { status: "absent" } as const; + return undefined; } throw error; } @@ -285,7 +278,7 @@ async function runLambdaMutation(mutation: () => Promise): Promise { return await mutation(); } catch (error) { if (isIamPropagationFailure(error)) { - throw new RetryableResourceError("Waiting for IAM role to propagate", { + throw new ResourceNotReadyError("Waiting for IAM role to propagate", { cause: error, }); } diff --git a/packages/cli/src/plan.ts b/packages/cli/src/plan.ts index ef52ec8..cc48067 100644 --- a/packages/cli/src/plan.ts +++ b/packages/cli/src/plan.ts @@ -19,6 +19,7 @@ const decisionSymbols: Record = { "drift-update": "~", "drift-recreate": "±", "delete-orphan": "-", + indeterminate: "?", noop: " ", }; @@ -78,6 +79,7 @@ function printPlanSummary(result: Plan, logger: Logger) { `${count("create")} to create`, `${count("update") + count("drift-update")} to update`, `${count("drift-recreate")} to recreate`, + `${count("indeterminate")} indeterminate`, `${count("delete-orphan")} to delete`, `${count("noop")} unchanged`, ].join(", "); diff --git a/packages/core/src/orchestrator/resource.ts b/packages/core/src/orchestrator/resource.ts index e62eba6..80734d8 100644 --- a/packages/core/src/orchestrator/resource.ts +++ b/packages/core/src/orchestrator/resource.ts @@ -1,7 +1,6 @@ export type { BaseResource, ResourceType, - ResourceReadResult, ResourceOpts, DefineResourceMeta, DefineResourceApiSchema, @@ -16,7 +15,7 @@ export type { export { Resource, - RetryableResourceError, + ResourceNotReadyError, defineResource, resource, } from "@notation/resource"; diff --git a/packages/core/test/orchestrator/resource.doubles.ts b/packages/core/test/orchestrator/resource.doubles.ts index f086c66..a516ed5 100644 --- a/packages/core/test/orchestrator/resource.doubles.ts +++ b/packages/core/test/orchestrator/resource.doubles.ts @@ -66,12 +66,9 @@ export const testOperations = { async update() {}, async read() { return { - status: "found", - output: { - primaryKey: "", - optionalSecondaryKey: "", - requiredParam: "", - }, + primaryKey: "", + optionalSecondaryKey: "", + requiredParam: "", } as const; }, deriveParams() { diff --git a/packages/core/test/provisioner/operation.create.test.ts b/packages/core/test/provisioner/operation.create.test.ts index f0b990d..492c6d6 100644 --- a/packages/core/test/provisioner/operation.create.test.ts +++ b/packages/core/test/provisioner/operation.create.test.ts @@ -17,10 +17,7 @@ describe("resource creation", () => { const stateBackend = new MemoryStateBackend(); const readResult = { ...testResourceOutput, volatileComputed: "123" }; const createMock = vi.fn(async () => ({ primaryKey: "" })); - const readMock = vi.fn(async () => ({ - status: "found" as const, - output: readResult, - })); + const readMock = vi.fn(async () => readResult); const TestResource = TestResourceSchema.defineOperations({ ...testOperations, diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index ecb6038..5f2bddd 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -1,5 +1,5 @@ import { RetryableError, createWorkflow } from "yieldstar"; -import { RetryableResourceError } from "@notation/resource"; +import { ResourceNotReadyError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type CreateResourceParams, @@ -29,7 +29,7 @@ export async function* createResourceOperation( try { return await params.resource.create(resourceParams); } catch (err) { - if (err instanceof RetryableResourceError) { + if (ResourceNotReadyError.is(err)) { throw new RetryableError(err.message, { ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), }); @@ -54,14 +54,14 @@ export async function* createResourceOperation( retryAbsent: true, }); - if (readResult.status !== "found") { + if (!readResult) { throw new Error( "Post-create read completed without finding the resource", ); } params.resource.setOutput({ ...params.resource.output, - ...readResult.output, + ...readResult, }); yield* step.run("create:persist-state", async () => { diff --git a/packages/reconciler/src/operations/operation.delete.ts b/packages/reconciler/src/operations/operation.delete.ts index 39a446a..ebc0bfa 100644 --- a/packages/reconciler/src/operations/operation.delete.ts +++ b/packages/reconciler/src/operations/operation.delete.ts @@ -1,5 +1,5 @@ import { RetryableError, createWorkflow } from "yieldstar"; -import { RetryableResourceError } from "@notation/resource"; +import { ResourceNotReadyError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type DeleteResourceParams, @@ -27,7 +27,7 @@ export async function* deleteResourceOperation( params.resource.toState(params.resource.output), ); } catch (err) { - if (err instanceof RetryableResourceError) { + if (ResourceNotReadyError.is(err)) { throw new RetryableError(err.message, { ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), }); diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 8eaf630..827903e 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -1,4 +1,5 @@ import { RetryableError, createWorkflow } from "yieldstar"; +import { ResourceNotReadyError } from "@notation/resource"; import { DEFAULT_READ_POLL_OPTIONS, type ReadResourceParams, @@ -7,18 +8,15 @@ import { getErrorDetails, } from "./operation.types"; -export type SettledResourceReadResult = - { status: "found"; output: Record } | { status: "absent" }; - export async function* readResourceOperation( step: StepRunner, params: ReadResourceParams, -): AsyncGenerator { +): AsyncGenerator | undefined, unknown> { await emitLifecycleEvent(params, "read", "start"); if (params.dryRun) { await emitLifecycleEvent(params, "read", "dry-run"); - return { status: "found", output: {} }; + return {}; } try { @@ -38,41 +36,48 @@ export async function* readResourceOperation( reason: "read-not-implemented", }); await emitLifecycleEvent(params, "read", "success"); - return { - status: "found", - output: merged as Record, - }; + return merged as Record; } const remote = yield* step.run("read:remote", async () => { - const result = await params.resource.read!(params.resource.key); - if (result.status === "pending") { - throw new RetryableError(result.reason, { - ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - if (result.status === "absent" && params.retryAbsent) { - throw new RetryableError("Waiting for resource to become visible", { - ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); + try { + const output = await params.resource.read!(params.resource.key); + + if (output === undefined && params.retryAbsent) { + throw new RetryableError("Waiting for resource to become visible", { + ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + + // Absence is `null` rather than `undefined` so that it survives the + // step's JSON round-trip when the run is replayed. + return output ?? null; + } catch (err) { + // A tagged not-ready condition is the provider telling us to wait. + // Everything else is a genuine failure and must surface. + if (params.retryNotReady !== false && ResourceNotReadyError.is(err)) { + throw new RetryableError(err.message, { + ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), + }); + } + throw err; } - return result; }); - if (remote.status === "absent") { + if (remote === null) { await emitLifecycleEvent(params, "read", "skip", { reason: "resource-absent", }); - return remote; + return undefined; } const mergedOutput = { ...resourceParams, - ...remote.output, + ...remote, }; await emitLifecycleEvent(params, "read", "success"); - return { status: "found", output: mergedOutput }; + return mergedOutput; } catch (err) { await emitLifecycleEvent(params, "read", "error", getErrorDetails(err)); throw err; diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index f27dc40..5a52b5d 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -61,6 +61,8 @@ export type CreateResourceParams = ResourceOperationBaseParams & { export type ReadResourceParams = ResourceOperationBaseParams & { retryAbsent?: boolean; + /** Defaults to true; planning sets it false so it can decide instead. */ + retryNotReady?: boolean; }; export type UpdateResourceParams = ResourceOperationBaseParams & { diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index 95f85a5..d8878d6 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -1,5 +1,5 @@ import { RetryableError, createWorkflow } from "yieldstar"; -import { RetryableResourceError } from "@notation/resource"; +import { ResourceNotReadyError } from "@notation/resource"; import { DEFAULT_RETRY_OPTIONS, type StepRunner, @@ -42,7 +42,7 @@ export async function* updateResourceOperation( params.resource.toState(params.resource.output), ); } catch (err) { - if (err instanceof RetryableResourceError) { + if (ResourceNotReadyError.is(err)) { throw new RetryableError(err.message, { ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), }); @@ -64,14 +64,14 @@ export async function* updateResourceOperation( retryAbsent: true, }); - if (readResult.status !== "found") { + if (!readResult) { throw new Error( "Post-update read completed without finding the resource", ); } params.resource.setOutput({ ...params.resource.output, - ...readResult.output, + ...readResult, }); yield* step.run("update:persist-state", async () => { diff --git a/packages/reconciler/src/plan.ts b/packages/reconciler/src/plan.ts index c9ec40c..6b575d3 100644 --- a/packages/reconciler/src/plan.ts +++ b/packages/reconciler/src/plan.ts @@ -12,6 +12,7 @@ export type PlanDecision = | "drift-update" | "drift-recreate" | "delete-orphan" + | "indeterminate" | "noop"; export type PlanDiff = { @@ -24,6 +25,7 @@ export type PlanNode = { id: string; type: string; decision: PlanDecision; + reason?: string; diff?: PlanDiff; params: Record; dependsOn: string[]; @@ -34,14 +36,21 @@ export type Plan = { nodes: PlanNode[]; }; +/** + * What a drift read told us about the remote. `not-ready` carries the message + * from the provider's {@link ResourceNotReadyError}: planning cannot compare + * against a resource that has not settled, so it reports rather than guesses. + */ export type DriftRead = - | { status: "found"; output: Record } - | { status: "not-found" }; + | { kind: "present"; output: Record } + | { kind: "absent" } + | { kind: "not-ready"; reason: string }; export type ResourceAction = | { decision: "create" } | { decision: "noop" } | { decision: "drift-recreate" } + | { decision: "indeterminate"; reason: string } | { decision: "update"; patch: Record; diff: PlanDiff } | { decision: "drift-update"; @@ -64,7 +73,11 @@ export function decideAction(opts: { >; if (driftRead) { - if (driftRead.status === "not-found") { + if (driftRead.kind === "not-ready") { + return { decision: "indeterminate", reason: driftRead.reason }; + } + + if (driftRead.kind === "absent") { return { decision: stateNode ? "drift-recreate" : "create" }; } diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index ffcfd60..d273da8 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -1,3 +1,4 @@ +import { ResourceNotReadyError } from "@notation/resource"; import type { BaseResource, ResourceType } from "@notation/resource"; import { RevConflict, type State, type StateNode } from "@notation/state"; import { RetryableError } from "yieldstar"; @@ -288,6 +289,8 @@ export class Reconciler { } } + assertDecidable(action, resource); + if (action.decision === "drift-update") { await this.#emit?.({ level: "info", @@ -360,7 +363,8 @@ export class Reconciler { params, driftRead: remote, }); - if (remote.status === "found") resource.setOutput(remote.output); + assertDecidable(action, resource); + if (remote.kind === "present") resource.setOutput(remote.output); await this.#emit?.({ level: "info", @@ -430,7 +434,9 @@ export class Reconciler { let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection) { - const driftRead = await this.#readForDrift(resource); + const driftRead = await this.#readForDrift(resource, { + retryNotReady: false, + }); action = decideAction({ resource, stateNode, params, driftRead }); } @@ -438,24 +444,34 @@ export class Reconciler { id: resource.id, type: resource.type, decision: action.decision, + ...("reason" in action ? { reason: action.reason } : {}), ...("diff" in action ? { diff: action.diff } : {}), params, dependsOn: getDependencyIds(resource), }; } - async #readForDrift(resource: BaseResource): Promise { - const result = await runOperation( - readResourceOperation(this.#stepRunner, { - resource, - state: this.#state, - emit: this.#emit, - readPollOptions: this.#readPollOptions, - }), - ); - return result.status === "found" - ? { status: "found", output: result.output } - : { status: "not-found" }; + async #readForDrift( + resource: BaseResource, + opts: { retryNotReady?: boolean } = {}, + ): Promise { + try { + const output = await runOperation( + readResourceOperation(this.#stepRunner, { + resource, + state: this.#state, + emit: this.#emit, + readPollOptions: this.#readPollOptions, + retryNotReady: opts.retryNotReady, + }), + ); + return output ? { kind: "present", output } : { kind: "absent" }; + } catch (err) { + if (opts.retryNotReady === false && ResourceNotReadyError.is(err)) { + return { kind: "not-ready", reason: err.message }; + } + throw err; + } } async #deleteOrphans( @@ -532,7 +548,7 @@ export class Reconciler { if (!resource.read) throw conflict; const remote = await this.#readForDrift(resource); - if (remote.status === "not-found") { + if (remote.kind !== "present") { if (!dryRun) await this.#state.delete(resource.id, stateNode.rev); return; } @@ -552,6 +568,20 @@ export class Reconciler { } } +/** + * Deploy reads retry not-ready conditions rather than reporting them, so a + * deploy decision is never indeterminate. Asserting it keeps the apply paths + * honest if that ever changes. + */ +function assertDecidable( + action: ResourceAction, + resource: BaseResource, +): asserts action is Exclude { + if (action.decision === "indeterminate") { + throw new Error(`Cannot deploy ${resource.id}: ${action.reason}`); + } +} + export async function runOperation( operation: AsyncGenerator, ) { diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index ac03552..e6eb306 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { RetryableError } from "yieldstar"; -import { resource, RetryableResourceError } from "@notation/resource"; +import { resource, ResourceNotReadyError } from "@notation/resource"; import { createResourceOperation, deleteResourceOperation, @@ -93,7 +93,7 @@ describe("operation workflows", () => { const createMock = vi.fn(async () => { createAttempts += 1; if (createAttempts === 1) { - throw new RetryableResourceError("retry create"); + throw new ResourceNotReadyError("retry create"); } return { remoteId: "abc" }; }); @@ -102,10 +102,7 @@ describe("operation workflows", () => { .defineSchema({}) .defineOperations({ create: createMock, - read: async () => ({ - status: "found", - output: { remoteId: "abc", status: "ready" }, - }), + read: async () => ({ remoteId: "abc", status: "ready" }), delete: async () => undefined, }); @@ -136,7 +133,7 @@ describe("operation workflows", () => { }); }); - it("read retries while the resource reports a pending outcome", async () => { + it("read retries while the resource reports a not-ready condition", async () => { const step = createStepRunnerDouble(); const state = { get: vi.fn(async () => undefined), @@ -152,15 +149,9 @@ describe("operation workflows", () => { read: async () => { readAttempts += 1; if (readAttempts < 3) { - return { - status: "pending", - reason: "resource is not ready", - } as const; + throw new ResourceNotReadyError("resource is not ready"); } - return { - status: "found", - output: { status: "ready" }, - } as const; + return { status: "ready" } as const; }, delete: async () => undefined, }); @@ -175,10 +166,7 @@ describe("operation workflows", () => { ); expect(readAttempts).toBe(3); - expect(result).toEqual({ - status: "found", - output: { status: "ready" }, - }); + expect(result).toEqual({ status: "ready" }); }); it("retries an absent read after creation until the resource is visible", async () => { @@ -195,11 +183,8 @@ describe("operation workflows", () => { create: async () => ({}), read: async () => { readAttempts += 1; - if (readAttempts === 1) return { status: "absent" } as const; - return { - status: "found", - output: { remoteId: "visible" }, - } as const; + if (readAttempts === 1) return undefined; + return { remoteId: "visible" } as const; }, delete: async () => undefined, }); diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index ed10726..2f6d359 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { resource, type ResourceReadResult } from "@notation/resource"; +import { resource } from "@notation/resource"; import { LeaseConflict, MemoryStateBackend, @@ -61,7 +61,7 @@ function createTestResourceClass(opts: { ) => Promise | void>; read?: ( key: Record, - ) => Promise>>; + ) => Promise | undefined>; update?: ( key: Record, patch: Record, @@ -90,8 +90,7 @@ function createTestResourceClass(opts: { } const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); -const found = (output: Record) => - ({ status: "found", output }) as const; +const found = (output: Record) => output; describe("reconciler deploy", () => { it("chooses create vs update from desired params vs state", async () => { @@ -543,7 +542,7 @@ describe("reconciler destroy + refresh", () => { remoteExists = false; }); const readSpy = vi.fn(async () => { - if (!remoteExists) return { status: "absent" } as const; + if (!remoteExists) return undefined; return found({ name: "doomed" }); }); const DestroyResource = createTestResourceClass({ diff --git a/packages/reconciler/test/reconciler.plan.test.ts b/packages/reconciler/test/reconciler.plan.test.ts index b3c1fd8..9723899 100644 --- a/packages/reconciler/test/reconciler.plan.test.ts +++ b/packages/reconciler/test/reconciler.plan.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it, vi } from "vitest"; import { + ResourceNotReadyError, resource, type BaseResource, - type ResourceReadResult, } from "@notation/resource"; import type { StateNode } from "@notation/state"; import { Reconciler, UNKNOWN_AFTER_APPLY } from "../src"; @@ -43,7 +43,7 @@ function createTestResourceClass(opts: { ) => Promise | void>; read?: ( key: Record, - ) => Promise>>; + ) => Promise | undefined>; update?: ( key: Record, patch: Record, @@ -172,10 +172,7 @@ describe("reconciler plan", () => { }); it("plans drift-update from live read output when drift detection is on", async () => { - const readSpy = vi.fn(async () => ({ - status: "found" as const, - output: { name: "drifted" }, - })); + const readSpy = vi.fn(async () => ({ name: "drifted" })); const TestResource = createTestResourceClass({ type: "test/service/plan-drift-update", read: readSpy, @@ -210,7 +207,7 @@ describe("reconciler plan", () => { it("plans drift-recreate when the remote resource is gone", async () => { const TestResource = createTestResourceClass({ type: "test/service/plan-drift-recreate", - read: async () => ({ status: "absent" }), + read: async () => undefined, }); const state = createMemoryState({ @@ -232,11 +229,34 @@ describe("reconciler plan", () => { }); }); + it("plans indeterminate when the remote reports a not-ready condition", async () => { + const TestResource = createTestResourceClass({ + type: "test/service/plan-not-ready", + read: async () => { + throw new ResourceNotReadyError("Waiting for Lambda to become active"); + }, + }); + + const state = createMemoryState({ + resource: createStateNode("resource", "test/service/plan-not-ready", { + name: "desired", + }), + }); + const reconciler = new Reconciler({ state, driftDetection: true }); + + const plan = await reconciler.plan([ + new TestResource({ id: "resource", config: { name: "desired" } }), + ]); + + expect(plan.nodes[0]).toMatchObject({ + id: "resource", + decision: "indeterminate", + reason: "Waiting for Lambda to become active", + }); + }); + it("skips remote reads when drift detection is off", async () => { - const readSpy = vi.fn(async () => ({ - status: "found" as const, - output: { name: "drifted" }, - })); + const readSpy = vi.fn(async () => ({ name: "drifted" })); const TestResource = createTestResourceClass({ type: "test/service/plan-no-read", read: readSpy, @@ -407,10 +427,7 @@ describe("reconciler plan", () => { create: createSpy, update: updateSpy, delete: deleteSpy, - read: async () => ({ - status: "found", - output: { name: "drifted" }, - }), + read: async () => ({ name: "drifted" }), }); const state = createMemoryState({ diff --git a/packages/resource/src/resource.ts b/packages/resource/src/resource.ts index bf1271e..0b42402 100644 --- a/packages/resource/src/resource.ts +++ b/packages/resource/src/resource.ts @@ -22,17 +22,29 @@ export type { Schema, SchemaItem, DefineResourceApiSchema }; export type ResourceType = `${string}/${string}/${string}`; -export type ResourceReadResult = - | { status: "found"; output: T } - | { status: "absent" } - | { status: "pending"; reason: string }; - -export class RetryableResourceError extends Error { - readonly code = "RESOURCE_RETRYABLE"; +/** + * Thrown by a resource operation when the provider reports a known temporary + * condition — the resource exists but is not yet usable, or a dependency has + * not finished propagating. + * + * Consumers recognise it by its declared `_tag`, not by class identity, so it + * survives being thrown across package or realm boundaries. + */ +export class ResourceNotReadyError extends Error { + readonly _tag = "ResourceNotReadyError"; constructor(message: string, options?: ErrorOptions) { super(message, options); - this.name = "RetryableResourceError"; + this.name = "ResourceNotReadyError"; + } + + static is(error: unknown): error is ResourceNotReadyError { + return ( + typeof error === "object" && + error !== null && + "_tag" in error && + (error as { _tag: unknown })._tag === "ResourceNotReadyError" + ); } } @@ -77,7 +89,7 @@ export interface BaseResource { readonly dependencies: Record; readonly key: {}; create: (params: any) => Promise<{} | void>; - read?: (key: any) => Promise>>; + read?: (key: any) => Promise | undefined>; update?: (key: any, patch: any, params: any, state: any) => Promise; delete: (key: any, state: any) => Promise; getParams(): Promise<{}>; @@ -105,9 +117,7 @@ export abstract class Resource< abstract type: ResourceType; abstract schema: Schema; abstract create: (params: T["params"]) => Promise; - abstract read?: ( - key: T["compoundKey"], - ) => Promise>; + abstract read?: (key: T["compoundKey"]) => Promise; abstract update?: ( key: T["compoundKey"], patch: T["params"], @@ -187,7 +197,7 @@ export type ResourceOperationsOptions< IntrinsicParams extends Partial, > = { create: (params: T["params"]) => Promise; - read?: (key: T["compoundKey"]) => Promise>; + read?: (key: T["compoundKey"]) => Promise; update?: ( key: T["compoundKey"], patch: T["params"], diff --git a/packages/resource/test/resource.doubles.ts b/packages/resource/test/resource.doubles.ts index a689c59..d52dd7f 100644 --- a/packages/resource/test/resource.doubles.ts +++ b/packages/resource/test/resource.doubles.ts @@ -66,12 +66,9 @@ export const testOperations = { async update() {}, async read() { return { - status: "found", - output: { - primaryKey: "", - optionalSecondaryKey: "", - requiredParam: "", - }, + primaryKey: "", + optionalSecondaryKey: "", + requiredParam: "", } as const; }, deriveParams() { diff --git a/packages/resource/test/resource.test.ts b/packages/resource/test/resource.test.ts index ca5722a..8b2163f 100644 --- a/packages/resource/test/resource.test.ts +++ b/packages/resource/test/resource.test.ts @@ -1,5 +1,5 @@ import { expect, it, test, vi } from "vitest"; -import { resource } from "src"; +import { ResourceNotReadyError, resource } from "src"; import { TestResource, testResourceConfig, @@ -138,3 +138,25 @@ describe("resource dependencies", () => { }); }); }); + +describe("ResourceNotReadyError", () => { + it("recognises its own instances", () => { + expect(ResourceNotReadyError.is(new ResourceNotReadyError("waiting"))).toBe( + true, + ); + }); + + it("recognises the tag without class identity", () => { + // A copy thrown from another realm or bundle carries the tag, not the class. + const fromElsewhere = Object.assign(new Error("waiting"), { + _tag: "ResourceNotReadyError", + }); + expect(ResourceNotReadyError.is(fromElsewhere)).toBe(true); + }); + + it("rejects unrelated errors", () => { + expect(ResourceNotReadyError.is(new Error("boom"))).toBe(false); + expect(ResourceNotReadyError.is({ _tag: "SomethingElse" })).toBe(false); + expect(ResourceNotReadyError.is(undefined)).toBe(false); + }); +}); diff --git a/packages/state/package.json b/packages/state/package.json index 8a49f6f..2c81b57 100644 --- a/packages/state/package.json +++ b/packages/state/package.json @@ -11,9 +11,6 @@ "build": "tsup --clean", "dev": "tsup --watch" }, - "dependencies": { - "@notation/utils": "workspace:*" - }, "devDependencies": { "@types/node": "^22.13.4" } diff --git a/packages/state/src/state.ts b/packages/state/src/state.ts index d0c10c1..003faa1 100644 --- a/packages/state/src/state.ts +++ b/packages/state/src/state.ts @@ -9,7 +9,6 @@ import { } from "node:fs/promises"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; -import { isErrorWithCode } from "@notation/utils"; import { LeaseConflict, RevConflict } from "./conflicts"; export type StateNode = { @@ -223,7 +222,7 @@ export class FileStateBackend implements StateBackend { }); break; } catch (error) { - if (!isErrorWithCode(error, "EEXIST")) throw error; + if (!isFileExistsError(error)) throw error; const current = await readFileLease(leaseFilePath); if (!current || current.expiresAtMs <= Date.now()) { await unlink(leaseFilePath).catch(() => undefined); @@ -272,7 +271,7 @@ export class FileStateBackend implements StateBackend { const file = await readFile(this.stateFilePath, "utf8"); return JSON.parse(file) as Record; } catch (error) { - if (isErrorWithCode(error, "ENOENT")) { + if (isFileMissingError(error)) { return {}; } @@ -299,7 +298,7 @@ export class FileStateBackend implements StateBackend { ); break; } catch (error) { - if (!isErrorWithCode(error, "EEXIST")) throw error; + if (!isFileExistsError(error)) throw error; const lockStat = await stat(lockFilePath).catch(() => undefined); if (lockStat && Date.now() - lockStat.mtimeMs > FILE_LOCK_STALE_MS) { await unlink(lockFilePath).catch(() => undefined); @@ -368,12 +367,29 @@ async function readFileLease( try { return JSON.parse(await readFile(filePath, "utf8")) as FileLeaseRecord; } catch (error) { - if (isErrorWithCode(error, "ENOENT") || error instanceof SyntaxError) + if (isFileMissingError(error) || error instanceof SyntaxError) return undefined; throw error; } } +function isFileMissingError(error: unknown): boolean { + return isErrorWithCode(error, "ENOENT"); +} + +function isFileExistsError(error: unknown): boolean { + return isErrorWithCode(error, "EEXIST"); +} + +function isErrorWithCode(error: unknown, code: string): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === code + ); +} + function cloneAsPersistedState( state: Record, ): Record { diff --git a/packages/std.iac/package.json b/packages/std.iac/package.json index 90d9789..63cfaa2 100644 --- a/packages/std.iac/package.json +++ b/packages/std.iac/package.json @@ -15,7 +15,6 @@ "dependencies": { "@notation/core": "workspace:*", "@notation/resource": "workspace:*", - "@notation/utils": "workspace:*", "fflate": "0.8.2" }, "devDependencies": { diff --git a/packages/std.iac/src/resources/fs/file.ts b/packages/std.iac/src/resources/fs/file.ts index 2bfd155..bf35031 100644 --- a/packages/std.iac/src/resources/fs/file.ts +++ b/packages/std.iac/src/resources/fs/file.ts @@ -1,5 +1,4 @@ import { resource } from "@notation/resource"; -import { isErrorWithCode } from "@notation/utils"; import { getSourceSha256 } from "src/utils/hash"; import * as fs from "node:fs/promises"; @@ -39,13 +38,10 @@ export const File = fileSchema.defineOperations({ read: async (config) => { try { const file = await fs.readFile(config.filePath); - return { - status: "found", - output: { ...config, file }, - } as const; + return { ...config, file }; } catch (error) { - if (isErrorWithCode(error, "ENOENT")) { - return { status: "absent" } as const; + if (isFileMissing(error)) { + return undefined; } throw error; } @@ -56,3 +52,12 @@ export const File = fileSchema.defineOperations({ }); export type FileInstance = InstanceType; + +function isFileMissing(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ); +} diff --git a/packages/std.iac/src/resources/fs/zip.ts b/packages/std.iac/src/resources/fs/zip.ts index 57d1caa..b4d1f09 100644 --- a/packages/std.iac/src/resources/fs/zip.ts +++ b/packages/std.iac/src/resources/fs/zip.ts @@ -1,5 +1,4 @@ import { resource } from "@notation/resource"; -import { isErrorWithCode } from "@notation/utils"; import * as fs from "node:fs/promises"; import { zip } from "src/utils/zip"; import { getSourceSha256 } from "src/utils/hash"; @@ -54,13 +53,10 @@ export const Zip = zipSchema.defineOperations({ read: async (params) => { try { const file = await fs.readFile(params.filePath); - return { - status: "found", - output: { ...params, file }, - } as const; + return { ...params, file }; } catch (error) { - if (isErrorWithCode(error, "ENOENT")) { - return { status: "absent" } as const; + if (isFileMissing(error)) { + return undefined; } throw error; } @@ -76,9 +72,18 @@ export const Zip = zipSchema.defineOperations({ try { await fs.unlink(config.filePath); } catch (error) { - if (!isErrorWithCode(error, "ENOENT")) throw error; + if (!isFileMissing(error)) throw error; } }, }); export type ZipFileInstance = InstanceType; + +function isFileMissing(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ENOENT" + ); +} diff --git a/packages/utils/package.json b/packages/utils/package.json deleted file mode 100644 index ef2ec23..0000000 --- a/packages/utils/package.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "type": "module", - "name": "@notation/utils", - "version": "0.12.0", - "main": "./dist/index.js", - "types": "./dist/index.d.ts", - "files": [ - "dist" - ], - "scripts": { - "build": "tsup --clean", - "dev": "tsup --watch", - "typecheck": "tsc --noEmit" - }, - "devDependencies": { - "@types/node": "^22.13.4" - } -} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts deleted file mode 100644 index fc8b93c..0000000 --- a/packages/utils/src/index.ts +++ /dev/null @@ -1,11 +0,0 @@ -export function isErrorWithCode( - error: unknown, - code: string, -): error is { code: string } { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === code - ); -} diff --git a/packages/utils/test/index.test.ts b/packages/utils/test/index.test.ts deleted file mode 100644 index 99a969a..0000000 --- a/packages/utils/test/index.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { isErrorWithCode } from "../src"; - -describe("isErrorWithCode", () => { - it("recognises an object with the requested code", () => { - expect(isErrorWithCode({ code: "ENOENT" }, "ENOENT")).toBe(true); - }); - - it("rejects other codes and non-object values", () => { - expect(isErrorWithCode({ code: "EEXIST" }, "ENOENT")).toBe(false); - expect(isErrorWithCode("ENOENT", "ENOENT")).toBe(false); - }); -}); diff --git a/packages/utils/tsconfig.json b/packages/utils/tsconfig.json deleted file mode 100644 index 4423630..0000000 --- a/packages/utils/tsconfig.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "extends": "tsconfig/base.json", - "compilerOptions": { - "baseUrl": ".", - "types": ["node"] - } -} diff --git a/packages/utils/tsup.config.ts b/packages/utils/tsup.config.ts deleted file mode 100644 index f0ac238..0000000 --- a/packages/utils/tsup.config.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { defineConfig } from "tsup"; - -export default defineConfig({ - entry: ["src/index.ts"], - dts: true, - format: ["esm"], -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 82d8d14..0b23b2f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -126,9 +126,6 @@ importers: '@notation/state-sqlite': specifier: workspace:* version: link:../../packages/state-sqlite - '@notation/utils': - specifier: workspace:* - version: link:../../packages/utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -355,10 +352,6 @@ importers: packages/resource: {} packages/state: - dependencies: - '@notation/utils': - specifier: workspace:* - version: link:../utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -382,9 +375,6 @@ importers: '@notation/resource': specifier: workspace:* version: link:../resource - '@notation/utils': - specifier: workspace:* - version: link:../utils fflate: specifier: 0.8.2 version: 0.8.2 @@ -395,12 +385,6 @@ importers: packages/tsconfig: {} - packages/utils: - devDependencies: - '@types/node': - specifier: ^22.13.4 - version: 22.13.4 - packages: '@aws-sdk/client-apigatewayv2@3.1080.0': From 8efe1a3a0f16e766594e59761216651e2a4fa8ab Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:02:58 +0100 Subject: [PATCH 4/6] Keep read policy at execution boundaries --- examples/reconciler/package.json | 3 +- examples/reconciler/src/static-site.ts | 12 +-- .../src/operations/operation.read.ts | 2 +- .../src/operations/operation.types.ts | 2 - packages/reconciler/src/plan.ts | 14 +--- packages/reconciler/src/reconciler.ts | 80 +++++++++---------- packages/state/package.json | 3 + packages/state/src/state.ts | 26 ++---- packages/std.iac/package.json | 1 + packages/std.iac/src/resources/fs/file.ts | 12 +-- packages/std.iac/src/resources/fs/zip.ts | 14 +--- packages/utils/package.json | 18 +++++ packages/utils/src/index.ts | 11 +++ packages/utils/test/index.test.ts | 13 +++ packages/utils/tsconfig.json | 7 ++ packages/utils/tsup.config.ts | 7 ++ pnpm-lock.yaml | 16 ++++ 17 files changed, 132 insertions(+), 109 deletions(-) create mode 100644 packages/utils/package.json create mode 100644 packages/utils/src/index.ts create mode 100644 packages/utils/test/index.test.ts create mode 100644 packages/utils/tsconfig.json create mode 100644 packages/utils/tsup.config.ts diff --git a/examples/reconciler/package.json b/examples/reconciler/package.json index 754b89a..55cbaa1 100644 --- a/examples/reconciler/package.json +++ b/examples/reconciler/package.json @@ -11,7 +11,8 @@ "dependencies": { "@notation/reconciler": "workspace:*", "@notation/resource": "workspace:*", - "@notation/state-sqlite": "workspace:*" + "@notation/state-sqlite": "workspace:*", + "@notation/utils": "workspace:*" }, "devDependencies": { "@types/node": "^22.13.4", diff --git a/examples/reconciler/src/static-site.ts b/examples/reconciler/src/static-site.ts index 6528a2b..511ce80 100644 --- a/examples/reconciler/src/static-site.ts +++ b/examples/reconciler/src/static-site.ts @@ -1,6 +1,7 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; type StaticSiteApi = { Key: { siteDirectory: string }; @@ -36,7 +37,7 @@ export const StaticSite = staticSite ); return { html }; } catch (error) { - if (isFileMissing(error)) { + if (isErrorWithCode(error, "ENOENT")) { return undefined; } throw error; @@ -49,12 +50,3 @@ export const StaticSite = staticSite await rm(siteDirectory, { recursive: true, force: true }); }, }); - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 827903e..0f17a5c 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -55,7 +55,7 @@ export async function* readResourceOperation( } catch (err) { // A tagged not-ready condition is the provider telling us to wait. // Everything else is a genuine failure and must surface. - if (params.retryNotReady !== false && ResourceNotReadyError.is(err)) { + if (ResourceNotReadyError.is(err)) { throw new RetryableError(err.message, { ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), }); diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index 5a52b5d..f27dc40 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -61,8 +61,6 @@ export type CreateResourceParams = ResourceOperationBaseParams & { export type ReadResourceParams = ResourceOperationBaseParams & { retryAbsent?: boolean; - /** Defaults to true; planning sets it false so it can decide instead. */ - retryNotReady?: boolean; }; export type UpdateResourceParams = ResourceOperationBaseParams & { diff --git a/packages/reconciler/src/plan.ts b/packages/reconciler/src/plan.ts index 6b575d3..35c14e1 100644 --- a/packages/reconciler/src/plan.ts +++ b/packages/reconciler/src/plan.ts @@ -36,21 +36,13 @@ export type Plan = { nodes: PlanNode[]; }; -/** - * What a drift read told us about the remote. `not-ready` carries the message - * from the provider's {@link ResourceNotReadyError}: planning cannot compare - * against a resource that has not settled, so it reports rather than guesses. - */ export type DriftRead = - | { kind: "present"; output: Record } - | { kind: "absent" } - | { kind: "not-ready"; reason: string }; + { kind: "present"; output: Record } | { kind: "absent" }; export type ResourceAction = | { decision: "create" } | { decision: "noop" } | { decision: "drift-recreate" } - | { decision: "indeterminate"; reason: string } | { decision: "update"; patch: Record; diff: PlanDiff } | { decision: "drift-update"; @@ -73,10 +65,6 @@ export function decideAction(opts: { >; if (driftRead) { - if (driftRead.kind === "not-ready") { - return { decision: "indeterminate", reason: driftRead.reason }; - } - if (driftRead.kind === "absent") { return { decision: stateNode ? "drift-recreate" : "create" }; } diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index d273da8..518f667 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -289,8 +289,6 @@ export class Reconciler { } } - assertDecidable(action, resource); - if (action.decision === "drift-update") { await this.#emit?.({ level: "info", @@ -363,7 +361,6 @@ export class Reconciler { params, driftRead: remote, }); - assertDecidable(action, resource); if (remote.kind === "present") resource.setOutput(remote.output); await this.#emit?.({ @@ -434,44 +431,61 @@ export class Reconciler { let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection) { - const driftRead = await this.#readForDrift(resource, { - retryNotReady: false, - }); - action = decideAction({ resource, stateNode, params, driftRead }); + try { + const driftRead = await this.#readForPlan(resource, params); + action = decideAction({ resource, stateNode, params, driftRead }); + } catch (error) { + if (!ResourceNotReadyError.is(error)) throw error; + return { + id: resource.id, + type: resource.type, + decision: "indeterminate", + reason: error.message, + params, + dependsOn: getDependencyIds(resource), + }; + } } return { id: resource.id, type: resource.type, decision: action.decision, - ...("reason" in action ? { reason: action.reason } : {}), ...("diff" in action ? { diff: action.diff } : {}), params, dependsOn: getDependencyIds(resource), }; } - async #readForDrift( + async #readForPlan( resource: BaseResource, - opts: { retryNotReady?: boolean } = {}, + params: Record, ): Promise { - try { - const output = await runOperation( - readResourceOperation(this.#stepRunner, { - resource, - state: this.#state, - emit: this.#emit, - readPollOptions: this.#readPollOptions, - retryNotReady: opts.retryNotReady, - }), - ); - return output ? { kind: "present", output } : { kind: "absent" }; - } catch (err) { - if (opts.retryNotReady === false && ResourceNotReadyError.is(err)) { - return { kind: "not-ready", reason: err.message }; - } - throw err; + if (!resource.read) { + return { + kind: "present", + output: { ...resource.output, ...params }, + }; } + + const output = await resource.read(resource.key); + return output === undefined + ? { kind: "absent" } + : { kind: "present", output: { ...params, ...output } }; + } + + async #readForDrift(resource: BaseResource): Promise { + const output = await runOperation( + readResourceOperation(this.#stepRunner, { + resource, + state: this.#state, + emit: this.#emit, + readPollOptions: this.#readPollOptions, + }), + ); + return output === undefined + ? { kind: "absent" } + : { kind: "present", output }; } async #deleteOrphans( @@ -568,20 +582,6 @@ export class Reconciler { } } -/** - * Deploy reads retry not-ready conditions rather than reporting them, so a - * deploy decision is never indeterminate. Asserting it keeps the apply paths - * honest if that ever changes. - */ -function assertDecidable( - action: ResourceAction, - resource: BaseResource, -): asserts action is Exclude { - if (action.decision === "indeterminate") { - throw new Error(`Cannot deploy ${resource.id}: ${action.reason}`); - } -} - export async function runOperation( operation: AsyncGenerator, ) { diff --git a/packages/state/package.json b/packages/state/package.json index 2c81b57..8a49f6f 100644 --- a/packages/state/package.json +++ b/packages/state/package.json @@ -11,6 +11,9 @@ "build": "tsup --clean", "dev": "tsup --watch" }, + "dependencies": { + "@notation/utils": "workspace:*" + }, "devDependencies": { "@types/node": "^22.13.4" } diff --git a/packages/state/src/state.ts b/packages/state/src/state.ts index 003faa1..d0c10c1 100644 --- a/packages/state/src/state.ts +++ b/packages/state/src/state.ts @@ -9,6 +9,7 @@ import { } from "node:fs/promises"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; +import { isErrorWithCode } from "@notation/utils"; import { LeaseConflict, RevConflict } from "./conflicts"; export type StateNode = { @@ -222,7 +223,7 @@ export class FileStateBackend implements StateBackend { }); break; } catch (error) { - if (!isFileExistsError(error)) throw error; + if (!isErrorWithCode(error, "EEXIST")) throw error; const current = await readFileLease(leaseFilePath); if (!current || current.expiresAtMs <= Date.now()) { await unlink(leaseFilePath).catch(() => undefined); @@ -271,7 +272,7 @@ export class FileStateBackend implements StateBackend { const file = await readFile(this.stateFilePath, "utf8"); return JSON.parse(file) as Record; } catch (error) { - if (isFileMissingError(error)) { + if (isErrorWithCode(error, "ENOENT")) { return {}; } @@ -298,7 +299,7 @@ export class FileStateBackend implements StateBackend { ); break; } catch (error) { - if (!isFileExistsError(error)) throw error; + if (!isErrorWithCode(error, "EEXIST")) throw error; const lockStat = await stat(lockFilePath).catch(() => undefined); if (lockStat && Date.now() - lockStat.mtimeMs > FILE_LOCK_STALE_MS) { await unlink(lockFilePath).catch(() => undefined); @@ -367,29 +368,12 @@ async function readFileLease( try { return JSON.parse(await readFile(filePath, "utf8")) as FileLeaseRecord; } catch (error) { - if (isFileMissingError(error) || error instanceof SyntaxError) + if (isErrorWithCode(error, "ENOENT") || error instanceof SyntaxError) return undefined; throw error; } } -function isFileMissingError(error: unknown): boolean { - return isErrorWithCode(error, "ENOENT"); -} - -function isFileExistsError(error: unknown): boolean { - return isErrorWithCode(error, "EEXIST"); -} - -function isErrorWithCode(error: unknown, code: string): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === code - ); -} - function cloneAsPersistedState( state: Record, ): Record { diff --git a/packages/std.iac/package.json b/packages/std.iac/package.json index 63cfaa2..90d9789 100644 --- a/packages/std.iac/package.json +++ b/packages/std.iac/package.json @@ -15,6 +15,7 @@ "dependencies": { "@notation/core": "workspace:*", "@notation/resource": "workspace:*", + "@notation/utils": "workspace:*", "fflate": "0.8.2" }, "devDependencies": { diff --git a/packages/std.iac/src/resources/fs/file.ts b/packages/std.iac/src/resources/fs/file.ts index bf35031..5cc42fc 100644 --- a/packages/std.iac/src/resources/fs/file.ts +++ b/packages/std.iac/src/resources/fs/file.ts @@ -1,4 +1,5 @@ import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; import { getSourceSha256 } from "src/utils/hash"; import * as fs from "node:fs/promises"; @@ -40,7 +41,7 @@ export const File = fileSchema.defineOperations({ const file = await fs.readFile(config.filePath); return { ...config, file }; } catch (error) { - if (isFileMissing(error)) { + if (isErrorWithCode(error, "ENOENT")) { return undefined; } throw error; @@ -52,12 +53,3 @@ export const File = fileSchema.defineOperations({ }); export type FileInstance = InstanceType; - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/std.iac/src/resources/fs/zip.ts b/packages/std.iac/src/resources/fs/zip.ts index b4d1f09..031d547 100644 --- a/packages/std.iac/src/resources/fs/zip.ts +++ b/packages/std.iac/src/resources/fs/zip.ts @@ -1,4 +1,5 @@ import { resource } from "@notation/resource"; +import { isErrorWithCode } from "@notation/utils"; import * as fs from "node:fs/promises"; import { zip } from "src/utils/zip"; import { getSourceSha256 } from "src/utils/hash"; @@ -55,7 +56,7 @@ export const Zip = zipSchema.defineOperations({ const file = await fs.readFile(params.filePath); return { ...params, file }; } catch (error) { - if (isFileMissing(error)) { + if (isErrorWithCode(error, "ENOENT")) { return undefined; } throw error; @@ -72,18 +73,9 @@ export const Zip = zipSchema.defineOperations({ try { await fs.unlink(config.filePath); } catch (error) { - if (!isFileMissing(error)) throw error; + if (!isErrorWithCode(error, "ENOENT")) throw error; } }, }); export type ZipFileInstance = InstanceType; - -function isFileMissing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ); -} diff --git a/packages/utils/package.json b/packages/utils/package.json new file mode 100644 index 0000000..ef2ec23 --- /dev/null +++ b/packages/utils/package.json @@ -0,0 +1,18 @@ +{ + "type": "module", + "name": "@notation/utils", + "version": "0.12.0", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "files": [ + "dist" + ], + "scripts": { + "build": "tsup --clean", + "dev": "tsup --watch", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "@types/node": "^22.13.4" + } +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts new file mode 100644 index 0000000..fc8b93c --- /dev/null +++ b/packages/utils/src/index.ts @@ -0,0 +1,11 @@ +export function isErrorWithCode( + error: unknown, + code: string, +): error is { code: string } { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === code + ); +} diff --git a/packages/utils/test/index.test.ts b/packages/utils/test/index.test.ts new file mode 100644 index 0000000..99a969a --- /dev/null +++ b/packages/utils/test/index.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from "vitest"; +import { isErrorWithCode } from "../src"; + +describe("isErrorWithCode", () => { + it("recognises an object with the requested code", () => { + expect(isErrorWithCode({ code: "ENOENT" }, "ENOENT")).toBe(true); + }); + + it("rejects other codes and non-object values", () => { + expect(isErrorWithCode({ code: "EEXIST" }, "ENOENT")).toBe(false); + expect(isErrorWithCode("ENOENT", "ENOENT")).toBe(false); + }); +}); diff --git a/packages/utils/tsconfig.json b/packages/utils/tsconfig.json new file mode 100644 index 0000000..4423630 --- /dev/null +++ b/packages/utils/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "tsconfig/base.json", + "compilerOptions": { + "baseUrl": ".", + "types": ["node"] + } +} diff --git a/packages/utils/tsup.config.ts b/packages/utils/tsup.config.ts new file mode 100644 index 0000000..f0ac238 --- /dev/null +++ b/packages/utils/tsup.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from "tsup"; + +export default defineConfig({ + entry: ["src/index.ts"], + dts: true, + format: ["esm"], +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0b23b2f..82d8d14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -126,6 +126,9 @@ importers: '@notation/state-sqlite': specifier: workspace:* version: link:../../packages/state-sqlite + '@notation/utils': + specifier: workspace:* + version: link:../../packages/utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -352,6 +355,10 @@ importers: packages/resource: {} packages/state: + dependencies: + '@notation/utils': + specifier: workspace:* + version: link:../utils devDependencies: '@types/node': specifier: ^22.13.4 @@ -375,6 +382,9 @@ importers: '@notation/resource': specifier: workspace:* version: link:../resource + '@notation/utils': + specifier: workspace:* + version: link:../utils fflate: specifier: 0.8.2 version: 0.8.2 @@ -385,6 +395,12 @@ importers: packages/tsconfig: {} + packages/utils: + devDependencies: + '@types/node': + specifier: ^22.13.4 + version: 22.13.4 + packages: '@aws-sdk/client-apigatewayv2@3.1080.0': From 62b07b4788f252c8e66a57170dddb9c17506cc25 Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:45:18 +0100 Subject: [PATCH 5/6] Define explicit resource operation signals --- .changeset/resource-operation-outcomes.md | 2 +- docs/internals/reconciler.md | 11 +- docs/internals/resource.md | 14 +- examples/reconciler/src/static-site.ts | 6 +- .../aws.iac/src/resources/api-gateway/api.ts | 6 +- .../aws.iac/src/resources/api-gateway/auth.ts | 7 +- .../api-gateway/lambda-integration.ts | 7 +- .../src/resources/api-gateway/route.ts | 6 +- .../src/resources/api-gateway/stage.ts | 6 +- .../src/resources/event-bridge/rule.ts | 6 +- .../src/resources/lambda/lambda-role.ts | 6 +- .../aws.iac/src/resources/lambda/lambda.ts | 26 +++- packages/cli/src/plan.ts | 2 - packages/core/src/orchestrator/resource.ts | 5 +- .../test/provisioner/operation.create.test.ts | 2 +- packages/reconciler/src/operations/index.ts | 1 + .../src/operations/operation.create.ts | 31 ++-- .../src/operations/operation.delete.ts | 27 ++-- .../src/operations/operation.pending.ts | 43 ++++++ .../src/operations/operation.read.ts | 44 ++---- .../src/operations/operation.types.ts | 31 +--- .../src/operations/operation.update.ts | 35 ++--- packages/reconciler/src/plan.ts | 2 - packages/reconciler/src/reconciler.ts | 117 ++++---------- .../test/operation.workflows.test.ts | 145 +++++++++++------- .../reconciler/test/reconciler.deploy.test.ts | 10 +- .../reconciler/test/reconciler.plan.test.ts | 27 ++-- packages/resource/src/index.ts | 1 + packages/resource/src/resource-operation.ts | 60 ++++++++ packages/resource/src/resource.ts | 85 +++++----- packages/resource/test/resource.test.ts | 53 +++++-- packages/std.iac/src/resources/fs/file.ts | 4 +- packages/std.iac/src/resources/fs/zip.ts | 6 +- 33 files changed, 452 insertions(+), 382 deletions(-) create mode 100644 packages/reconciler/src/operations/operation.pending.ts create mode 100644 packages/resource/src/resource-operation.ts diff --git a/.changeset/resource-operation-outcomes.md b/.changeset/resource-operation-outcomes.md index b7ccedc..9317acb 100644 --- a/.changeset/resource-operation-outcomes.md +++ b/.changeset/resource-operation-outcomes.md @@ -7,4 +7,4 @@ "@notation/std.iac": minor --- -A resource `read` now returns the remote object, or `undefined` when it does not exist. Providers translate their own not-found exceptions at the boundary. Known temporary conditions — a Lambda that is still deploying, an IAM role that has not propagated — throw the tagged `ResourceNotReadyError`, which the reconciler retries during deploys and reports as an `indeterminate` plan decision. +A resource `read` now returns the remote object or throws the tagged `ResourceNotFoundError`. Operations that have started but not settled throw `ResourceOperationPendingError` with their retry delay and optional callback context. The reconciler follows those explicit instructions instead of guessing retry behaviour from provider errors or call-site context. diff --git a/docs/internals/reconciler.md b/docs/internals/reconciler.md index 906273d..e37b4f5 100644 --- a/docs/internals/reconciler.md +++ b/docs/internals/reconciler.md @@ -81,18 +81,19 @@ Each CRUD operation is implemented as an async generator with retry support: - **`deleteResourceOperation`** – deletes the resource, removes the entry from state backend - **`readResourceOperation`** – reads current state from the cloud provider (used for drift detection) -### Retry and polling +### Pending operations -Operations support polling for eventual consistency: +A resource operation reports that it is still in progress by throwing `ResourceOperationPendingError` with a retry delay and optional callback context. The reconciler persists that context, waits for the requested delay, and invokes the operation again. It does not infer retry behaviour from provider errors. + +The reconciler limits the number of attempts as a safety boundary: ```ts [packages/reconciler/src/index.ts] { - maxAttempts: 10, - retryInterval: 2000, + maxOperationAttempts: 30, } ``` -This handles AWS services that return success before the resource is fully available. For example, after creating an IAM Role, a Lambda function may briefly fail to deploy until the role propagates. The retry loop handles cases like this. +Resources provide the timing because they understand the remote operation. For example, the Lambda resource reports IAM propagation and inactive function states as pending. ### Operation lifecycle diff --git a/docs/internals/resource.md b/docs/internals/resource.md index 25c94b4..fc0a61c 100644 --- a/docs/internals/resource.md +++ b/docs/internals/resource.md @@ -141,17 +141,17 @@ All schema items carry these fields: | Field | Required | Signature / Description | | -------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `create` | yes | `(params: Params) => Promise>` – create the resource, return its computed key. | -| `read` | no | `(key: CompoundKey) => Promise \| undefined>` – return the remote object, or `undefined` when it does not exist. | -| `update` | no | `(key, patch, params, state) => Promise` – apply a partial update. | -| `delete` | yes | `(key, state) => Promise` – ensure the resource is absent. Implementations must also succeed when the remote resource is already gone. | +| `create` | yes | `(params, context?) => Promise>` – create the resource, return its computed key. | +| `read` | no | `(key, context?) => Promise>` – return the remote object or throw `ResourceNotFoundError`. | +| `update` | no | `(key, patch, params, state, context?) => Promise` – apply a partial update. | +| `delete` | yes | `(key, state, context?) => Promise` – ensure the resource is absent. Implementations must also succeed when the remote resource is already gone. | | `deriveParams` | no | Computes intrinsic derived params from config (not dependency-aware). | -Resource operations translate provider-specific responses at the provider boundary. A read returns the remote object when it is found, and `undefined` when the provider says it does not exist — the provider's own not-found exception is caught and turned into `undefined` there, not further up the stack. +Resource operations translate provider-specific responses at the provider boundary. A read throws the structurally tagged `ResourceNotFoundError` when the provider says the resource does not exist. Delete implementations consume the equivalent provider error and complete successfully. -When the provider reports a known temporary condition — a Lambda that is still deploying, an IAM role that has not propagated — a read or a mutation throws `ResourceNotReadyError`. It is recognised by its declared `_tag` via `ResourceNotReadyError.is(error)`, never by provider name, message, or `instanceof`. Every other error propagates and fails the operation. +An operation that has started but not settled throws `ResourceOperationPendingError`. The error supplies the delay before the next attempt and may supply callback context for that attempt. The operation decides when this is appropriate: for example, a create handler may treat temporary absence as pending while an ordinary read reports not-found. -The reconciler decides what a not-ready condition means. Deploy, read, and mutation workflows retry it; planning reports it as an `indeterminate` decision carrying the error's message, because it cannot diff against a resource that has not settled. +The reconciler does not infer behaviour from provider error names, messages, or the operation being run. It persists callback context, waits for the requested delay, and invokes the same operation again. A configured maximum attempt count remains a framework safety limit. Every error other than the two declared resource-operation signals fails the operation. ## Dependencies diff --git a/examples/reconciler/src/static-site.ts b/examples/reconciler/src/static-site.ts index 511ce80..6e4eacf 100644 --- a/examples/reconciler/src/static-site.ts +++ b/examples/reconciler/src/static-site.ts @@ -1,6 +1,6 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { isErrorWithCode } from "@notation/utils"; type StaticSiteApi = { @@ -38,7 +38,9 @@ export const StaticSite = staticSite return { html }; } catch (error) { if (isErrorWithCode(error, "ENOENT")) { - return undefined; + throw new ResourceNotFoundError("Static site was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/api.ts b/packages/aws.iac/src/resources/api-gateway/api.ts index 492d27e..0e95ed7 100644 --- a/packages/aws.iac/src/resources/api-gateway/api.ts +++ b/packages/aws.iac/src/resources/api-gateway/api.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import * as sdk from "@aws-sdk/client-apigatewayv2"; import { apiGatewayClient } from "src/utils/aws-clients"; import { AwsSchema } from "src/utils/types"; @@ -105,7 +105,9 @@ export const Api = apiSchema.defineOperations({ return { RouteKey: "", ...result }; } catch (error) { if (error instanceof sdk.NotFoundException) { - return undefined; + throw new ResourceNotFoundError("API Gateway API was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/auth.ts b/packages/aws.iac/src/resources/api-gateway/auth.ts index 2f58785..f62e673 100644 --- a/packages/aws.iac/src/resources/api-gateway/auth.ts +++ b/packages/aws.iac/src/resources/api-gateway/auth.ts @@ -1,6 +1,6 @@ import * as sdk from "@aws-sdk/client-apigatewayv2"; import { AwsSchema } from "src/utils/types"; -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { apiGatewayClient } from "src/utils/aws-clients"; import { ApiInstance } from "./api"; @@ -65,7 +65,10 @@ export const RouteAuth = apiSchema return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return undefined; + throw new ResourceNotFoundError( + "API Gateway authorizer was not found", + { cause: error }, + ); } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts index 75d9367..a65f963 100644 --- a/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts +++ b/packages/aws.iac/src/resources/api-gateway/lambda-integration.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import * as sdk from "@aws-sdk/client-apigatewayv2"; import { ApiInstance } from "./api"; import { LambdaFunctionInstance } from "../lambda"; @@ -124,7 +124,10 @@ export const LambdaIntegration = integrationSchema return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return undefined; + throw new ResourceNotFoundError( + "API Gateway integration was not found", + { cause: error }, + ); } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/route.ts b/packages/aws.iac/src/resources/api-gateway/route.ts index 6252449..f22b9db 100644 --- a/packages/aws.iac/src/resources/api-gateway/route.ts +++ b/packages/aws.iac/src/resources/api-gateway/route.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import * as sdk from "@aws-sdk/client-apigatewayv2"; import { apiGatewayClient } from "src/utils/aws-clients"; import { ApiInstance, LambdaIntegrationInstance } from "."; @@ -98,7 +98,9 @@ export const Route = routeSchema return { ...key, ...result }; } catch (error) { if (error instanceof sdk.NotFoundException) { - return undefined; + throw new ResourceNotFoundError("API Gateway route was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/api-gateway/stage.ts b/packages/aws.iac/src/resources/api-gateway/stage.ts index 73f37d6..898bac2 100644 --- a/packages/aws.iac/src/resources/api-gateway/stage.ts +++ b/packages/aws.iac/src/resources/api-gateway/stage.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import * as sdk from "@aws-sdk/client-apigatewayv2"; import { ApiInstance } from "./api"; import { apiGatewayClient } from "src/utils/aws-clients"; @@ -82,7 +82,9 @@ export const Stage = stageSchema return output; } catch (error) { if (error instanceof sdk.NotFoundException) { - return undefined; + throw new ResourceNotFoundError("API Gateway stage was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/event-bridge/rule.ts b/packages/aws.iac/src/resources/event-bridge/rule.ts index d423567..66bf699 100644 --- a/packages/aws.iac/src/resources/event-bridge/rule.ts +++ b/packages/aws.iac/src/resources/event-bridge/rule.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { AwsSchema } from "src/utils/types"; import * as sdk from "@aws-sdk/client-eventbridge"; import { eventBridgeClient } from "src/utils/aws-clients"; @@ -79,7 +79,9 @@ export const EventBridgeRule = eventBridgeRuleSchema }; } catch (error) { if (error instanceof sdk.ResourceNotFoundException) { - return undefined; + throw new ResourceNotFoundError("EventBridge rule was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/lambda/lambda-role.ts b/packages/aws.iac/src/resources/lambda/lambda-role.ts index 309712d..27933e9 100644 --- a/packages/aws.iac/src/resources/lambda/lambda-role.ts +++ b/packages/aws.iac/src/resources/lambda/lambda-role.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import * as sdk from "@aws-sdk/client-iam"; import { iamClient } from "src/utils/aws-clients"; import { AwsSchema } from "src/utils/types"; @@ -75,7 +75,9 @@ export const LambdaIamRole = lambdaIamRoleSchema.defineOperations({ return Role!; } catch (error) { if (error instanceof sdk.NoSuchEntityException) { - return undefined; + throw new ResourceNotFoundError("Lambda IAM role was not found", { + cause: error, + }); } throw error; } diff --git a/packages/aws.iac/src/resources/lambda/lambda.ts b/packages/aws.iac/src/resources/lambda/lambda.ts index 19cafa8..08f759e 100644 --- a/packages/aws.iac/src/resources/lambda/lambda.ts +++ b/packages/aws.iac/src/resources/lambda/lambda.ts @@ -1,4 +1,9 @@ -import { resource, ResourceNotReadyError, typed } from "@notation/resource"; +import { + resource, + ResourceNotFoundError, + ResourceOperationPendingError, + typed, +} from "@notation/resource"; import * as sdk from "@aws-sdk/client-lambda"; import { lambdaClient } from "src/utils/aws-clients"; import { AwsSchema } from "src/utils/types"; @@ -204,12 +209,16 @@ export const LambdaFunction = lambdaFunctionSchema await lambdaClient.send(command); if (Configuration?.State !== "Active") { - throw new ResourceNotReadyError( + throw new ResourceOperationPendingError( "Waiting for Lambda to become active", + { retryAfterMs: 1_000 }, ); } if (!Configuration.RevisionId) { - throw new ResourceNotReadyError("Waiting for Lambda to be deployed"); + throw new ResourceOperationPendingError( + "Waiting for Lambda to be deployed", + { retryAfterMs: 1_000 }, + ); } return { @@ -225,7 +234,9 @@ export const LambdaFunction = lambdaFunctionSchema }; } catch (error) { if (error instanceof sdk.ResourceNotFoundException) { - return undefined; + throw new ResourceNotFoundError("Lambda function was not found", { + cause: error, + }); } throw error; } @@ -278,9 +289,10 @@ async function runLambdaMutation(mutation: () => Promise): Promise { return await mutation(); } catch (error) { if (isIamPropagationFailure(error)) { - throw new ResourceNotReadyError("Waiting for IAM role to propagate", { - cause: error, - }); + throw new ResourceOperationPendingError( + "Waiting for IAM role to propagate", + { retryAfterMs: 1_000, cause: error }, + ); } throw error; } diff --git a/packages/cli/src/plan.ts b/packages/cli/src/plan.ts index cc48067..ef52ec8 100644 --- a/packages/cli/src/plan.ts +++ b/packages/cli/src/plan.ts @@ -19,7 +19,6 @@ const decisionSymbols: Record = { "drift-update": "~", "drift-recreate": "±", "delete-orphan": "-", - indeterminate: "?", noop: " ", }; @@ -79,7 +78,6 @@ function printPlanSummary(result: Plan, logger: Logger) { `${count("create")} to create`, `${count("update") + count("drift-update")} to update`, `${count("drift-recreate")} to recreate`, - `${count("indeterminate")} indeterminate`, `${count("delete-orphan")} to delete`, `${count("noop")} unchanged`, ].join(", "); diff --git a/packages/core/src/orchestrator/resource.ts b/packages/core/src/orchestrator/resource.ts index 80734d8..6281556 100644 --- a/packages/core/src/orchestrator/resource.ts +++ b/packages/core/src/orchestrator/resource.ts @@ -9,13 +9,16 @@ export type { ResourceClass, ResourceSchemaBuilder, ResourceBuilder, + ResourceOperationContext, + ResourceOperationSignal, Schema, SchemaItem, } from "@notation/resource"; export { Resource, - ResourceNotReadyError, + ResourceNotFoundError, + ResourceOperationPendingError, defineResource, resource, } from "@notation/resource"; diff --git a/packages/core/test/provisioner/operation.create.test.ts b/packages/core/test/provisioner/operation.create.test.ts index 492c6d6..2e4f506 100644 --- a/packages/core/test/provisioner/operation.create.test.ts +++ b/packages/core/test/provisioner/operation.create.test.ts @@ -42,7 +42,7 @@ describe("resource creation", () => { const params = await testResource.getParams(); const persistedOutput = testResource.toState(readResult); - expect(createMock.mock.calls[0]).toEqual([params]); + expect(createMock.mock.calls[0]).toEqual([params, undefined]); await expect(stateBackend.get(testResource.id)).resolves.toMatchObject({ id: testResource.id, output: persistedOutput, diff --git a/packages/reconciler/src/operations/index.ts b/packages/reconciler/src/operations/index.ts index 04c66b6..b0107e7 100644 --- a/packages/reconciler/src/operations/index.ts +++ b/packages/reconciler/src/operations/index.ts @@ -1,4 +1,5 @@ export * from "./operation.types"; +export * from "./operation.pending"; export * from "./operation.create"; export * from "./operation.read"; export * from "./operation.update"; diff --git a/packages/reconciler/src/operations/operation.create.ts b/packages/reconciler/src/operations/operation.create.ts index 5f2bddd..e0aee4f 100644 --- a/packages/reconciler/src/operations/operation.create.ts +++ b/packages/reconciler/src/operations/operation.create.ts @@ -1,12 +1,11 @@ -import { RetryableError, createWorkflow } from "yieldstar"; -import { ResourceNotReadyError } from "@notation/resource"; +import { createWorkflow } from "yieldstar"; import { - DEFAULT_RETRY_OPTIONS, type CreateResourceParams, type StepRunner, emitLifecycleEvent, getErrorDetails, } from "./operation.types"; +import { runPendingOperation } from "./operation.pending"; import { readResourceOperation } from "./operation.read"; export async function* createResourceOperation( @@ -25,18 +24,12 @@ export async function* createResourceOperation( params.resource.getParams(), ); - const computedPrimaryKey = yield* step.run("create:remote", async () => { - try { - return await params.resource.create(resourceParams); - } catch (err) { - if (ResourceNotReadyError.is(err)) { - throw new RetryableError(err.message, { - ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), - }); - } - throw err; - } - }); + const computedPrimaryKey = yield* runPendingOperation( + step, + "create:remote", + (context) => params.resource.create(resourceParams, context), + params.maxOperationAttempts, + ); params.resource.setOutput(resourceParams); if (computedPrimaryKey) { @@ -50,15 +43,9 @@ export async function* createResourceOperation( resource: params.resource, state: params.state, emit: params.emit, - readPollOptions: params.readPollOptions, - retryAbsent: true, + maxOperationAttempts: params.maxOperationAttempts, }); - if (!readResult) { - throw new Error( - "Post-create read completed without finding the resource", - ); - } params.resource.setOutput({ ...params.resource.output, ...readResult, diff --git a/packages/reconciler/src/operations/operation.delete.ts b/packages/reconciler/src/operations/operation.delete.ts index ebc0bfa..51975b6 100644 --- a/packages/reconciler/src/operations/operation.delete.ts +++ b/packages/reconciler/src/operations/operation.delete.ts @@ -1,12 +1,11 @@ -import { RetryableError, createWorkflow } from "yieldstar"; -import { ResourceNotReadyError } from "@notation/resource"; +import { createWorkflow } from "yieldstar"; import { - DEFAULT_RETRY_OPTIONS, type DeleteResourceParams, type StepRunner, emitLifecycleEvent, getErrorDetails, } from "./operation.types"; +import { runPendingOperation } from "./operation.pending"; export async function* deleteResourceOperation( step: StepRunner, @@ -20,21 +19,17 @@ export async function* deleteResourceOperation( } try { - yield* step.run("delete:remote", async () => { - try { - await params.resource.delete( + yield* runPendingOperation( + step, + "delete:remote", + (context) => + params.resource.delete( params.resource.key, params.resource.toState(params.resource.output), - ); - } catch (err) { - if (ResourceNotReadyError.is(err)) { - throw new RetryableError(err.message, { - ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), - }); - } - throw err; - } - }); + context, + ), + params.maxOperationAttempts, + ); yield* step.run("delete:persist-state", () => params.state.delete(params.resource.id, params.expectedRev), diff --git a/packages/reconciler/src/operations/operation.pending.ts b/packages/reconciler/src/operations/operation.pending.ts new file mode 100644 index 0000000..04a6b7e --- /dev/null +++ b/packages/reconciler/src/operations/operation.pending.ts @@ -0,0 +1,43 @@ +import { + ResourceOperationPendingError, + type ResourceOperationContext, +} from "@notation/resource"; +import type { StepRunner } from "./operation.types"; + +export const DEFAULT_MAX_OPERATION_ATTEMPTS = 30; + +export async function* runPendingOperation( + step: StepRunner, + key: string, + operation: (context?: ResourceOperationContext) => T | Promise, + maxAttempts = DEFAULT_MAX_OPERATION_ATTEMPTS, +): AsyncGenerator { + if (!Number.isInteger(maxAttempts) || maxAttempts < 1) { + throw new RangeError("maxOperationAttempts must be a positive integer"); + } + + let context: ResourceOperationContext | undefined; + let pending: ResourceOperationPendingError | undefined; + + for (let attempt = 0; attempt < maxAttempts; attempt += 1) { + try { + return yield* step.run(`${key}:attempt:${attempt}`, () => + operation(context), + ); + } catch (error) { + if (!ResourceOperationPendingError.is(error)) throw error; + + pending = error; + context = error.callbackContext; + + if (attempt + 1 < maxAttempts) { + yield* step.delay(`${key}:retry-delay:${attempt}`, error.retryAfterMs); + } + } + } + + throw new Error( + `${pending?.message ?? "Resource operation remained pending"} after ${maxAttempts} attempts`, + { cause: pending }, + ); +} diff --git a/packages/reconciler/src/operations/operation.read.ts b/packages/reconciler/src/operations/operation.read.ts index 0f17a5c..e74196a 100644 --- a/packages/reconciler/src/operations/operation.read.ts +++ b/packages/reconciler/src/operations/operation.read.ts @@ -1,17 +1,16 @@ -import { RetryableError, createWorkflow } from "yieldstar"; -import { ResourceNotReadyError } from "@notation/resource"; +import { createWorkflow } from "yieldstar"; import { - DEFAULT_READ_POLL_OPTIONS, type ReadResourceParams, type StepRunner, emitLifecycleEvent, getErrorDetails, } from "./operation.types"; +import { runPendingOperation } from "./operation.pending"; export async function* readResourceOperation( step: StepRunner, params: ReadResourceParams, -): AsyncGenerator | undefined, unknown> { +): AsyncGenerator, unknown> { await emitLifecycleEvent(params, "read", "start"); if (params.dryRun) { @@ -39,37 +38,12 @@ export async function* readResourceOperation( return merged as Record; } - const remote = yield* step.run("read:remote", async () => { - try { - const output = await params.resource.read!(params.resource.key); - - if (output === undefined && params.retryAbsent) { - throw new RetryableError("Waiting for resource to become visible", { - ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - - // Absence is `null` rather than `undefined` so that it survives the - // step's JSON round-trip when the run is replayed. - return output ?? null; - } catch (err) { - // A tagged not-ready condition is the provider telling us to wait. - // Everything else is a genuine failure and must surface. - if (ResourceNotReadyError.is(err)) { - throw new RetryableError(err.message, { - ...(params.readPollOptions ?? DEFAULT_READ_POLL_OPTIONS), - }); - } - throw err; - } - }); - - if (remote === null) { - await emitLifecycleEvent(params, "read", "skip", { - reason: "resource-absent", - }); - return undefined; - } + const remote = yield* runPendingOperation( + step, + "read:remote", + (context) => params.resource.read!(params.resource.key, context), + params.maxOperationAttempts, + ); const mergedOutput = { ...resourceParams, diff --git a/packages/reconciler/src/operations/operation.types.ts b/packages/reconciler/src/operations/operation.types.ts index f27dc40..6cbcd1c 100644 --- a/packages/reconciler/src/operations/operation.types.ts +++ b/packages/reconciler/src/operations/operation.types.ts @@ -22,26 +22,12 @@ export type OperationEventEmitter = ( event: OperationLifecycleEvent, ) => void | Promise; -export type PollOptions = { - maxAttempts: number; - retryInterval: number; -}; - export type StepRunner = { run(fn: () => T | Promise): AsyncGenerator; run( key: string, fn: () => T | Promise, ): AsyncGenerator; - poll( - opts: PollOptions, - predicate: () => boolean | Promise, - ): AsyncGenerator; - poll( - key: string, - opts: PollOptions, - predicate: () => boolean | Promise, - ): AsyncGenerator; delay(ms: number): AsyncGenerator; delay(key: string, ms: number): AsyncGenerator; }; @@ -51,17 +37,14 @@ export type ResourceOperationBaseParams = { state: Pick; dryRun?: boolean; emit?: OperationEventEmitter; - retryOptions?: PollOptions; - readPollOptions?: PollOptions; + maxOperationAttempts?: number; }; export type CreateResourceParams = ResourceOperationBaseParams & { expectedRev: number; }; -export type ReadResourceParams = ResourceOperationBaseParams & { - retryAbsent?: boolean; -}; +export type ReadResourceParams = ResourceOperationBaseParams; export type UpdateResourceParams = ResourceOperationBaseParams & { patch: Record; @@ -72,16 +55,6 @@ export type DeleteResourceParams = ResourceOperationBaseParams & { expectedRev: number; }; -export const DEFAULT_RETRY_OPTIONS: PollOptions = { - maxAttempts: 10, - retryInterval: 1_000, -}; - -export const DEFAULT_READ_POLL_OPTIONS: PollOptions = { - maxAttempts: 30, - retryInterval: 1_000, -}; - export function getErrorDetails(err: unknown): { errorName: string; errorMessage: string; diff --git a/packages/reconciler/src/operations/operation.update.ts b/packages/reconciler/src/operations/operation.update.ts index d8878d6..fc62a8c 100644 --- a/packages/reconciler/src/operations/operation.update.ts +++ b/packages/reconciler/src/operations/operation.update.ts @@ -1,12 +1,11 @@ -import { RetryableError, createWorkflow } from "yieldstar"; -import { ResourceNotReadyError } from "@notation/resource"; +import { createWorkflow } from "yieldstar"; import { - DEFAULT_RETRY_OPTIONS, type StepRunner, type UpdateResourceParams, emitLifecycleEvent, getErrorDetails, } from "./operation.types"; +import { runPendingOperation } from "./operation.pending"; import { readResourceOperation } from "./operation.read"; export async function* updateResourceOperation( @@ -33,23 +32,19 @@ export async function* updateResourceOperation( params.resource.getParams(), ); - yield* step.run("update:remote", async () => { - try { - await params.resource.update!( + yield* runPendingOperation( + step, + "update:remote", + (context) => + params.resource.update!( params.resource.key, params.patch, resourceParams, params.resource.toState(params.resource.output), - ); - } catch (err) { - if (ResourceNotReadyError.is(err)) { - throw new RetryableError(err.message, { - ...(params.retryOptions ?? DEFAULT_RETRY_OPTIONS), - }); - } - throw err; - } - }); + context, + ), + params.maxOperationAttempts, + ); params.resource.setOutput({ ...params.resource.key, @@ -60,15 +55,9 @@ export async function* updateResourceOperation( resource: params.resource, state: params.state, emit: params.emit, - readPollOptions: params.readPollOptions, - retryAbsent: true, + maxOperationAttempts: params.maxOperationAttempts, }); - if (!readResult) { - throw new Error( - "Post-update read completed without finding the resource", - ); - } params.resource.setOutput({ ...params.resource.output, ...readResult, diff --git a/packages/reconciler/src/plan.ts b/packages/reconciler/src/plan.ts index 35c14e1..d2f8451 100644 --- a/packages/reconciler/src/plan.ts +++ b/packages/reconciler/src/plan.ts @@ -12,7 +12,6 @@ export type PlanDecision = | "drift-update" | "drift-recreate" | "delete-orphan" - | "indeterminate" | "noop"; export type PlanDiff = { @@ -25,7 +24,6 @@ export type PlanNode = { id: string; type: string; decision: PlanDecision; - reason?: string; diff?: PlanDiff; params: Record; dependsOn: string[]; diff --git a/packages/reconciler/src/reconciler.ts b/packages/reconciler/src/reconciler.ts index 518f667..fab6c0b 100644 --- a/packages/reconciler/src/reconciler.ts +++ b/packages/reconciler/src/reconciler.ts @@ -1,7 +1,6 @@ -import { ResourceNotReadyError } from "@notation/resource"; +import { ResourceNotFoundError } from "@notation/resource"; import type { BaseResource, ResourceType } from "@notation/resource"; import { RevConflict, type State, type StateNode } from "@notation/state"; -import { RetryableError } from "yieldstar"; import { setTimeout as sleep } from "node:timers/promises"; import { buildResourceDepthLevels } from "./dependency-graph"; import { @@ -18,7 +17,6 @@ import { deleteResourceOperation, readResourceOperation, type OperationLifecycleEvent, - type PollOptions, type StepRunner, updateResourceOperation, } from "./operations"; @@ -67,8 +65,7 @@ export type ReconcilerOptions = { dryRun?: boolean; driftDetection?: boolean; emit?: ReconcilerEventEmitter; - retryOptions?: PollOptions; - readPollOptions?: PollOptions; + maxOperationAttempts?: number; mutationLeaseTtl?: number; }; @@ -95,8 +92,7 @@ export class Reconciler { readonly #defaultDryRun: boolean; readonly #defaultDriftDetection: boolean; readonly #emit?: ReconcilerEventEmitter; - readonly #retryOptions?: PollOptions; - readonly #readPollOptions?: PollOptions; + readonly #maxOperationAttempts?: number; readonly #mutationLeaseTtl: number; readonly #stepRunner: StepRunner; @@ -106,8 +102,7 @@ export class Reconciler { this.#defaultDryRun = opts.dryRun ?? false; this.#defaultDriftDetection = opts.driftDetection ?? true; this.#emit = opts.emit; - this.#retryOptions = opts.retryOptions; - this.#readPollOptions = opts.readPollOptions; + this.#maxOperationAttempts = opts.maxOperationAttempts; this.#mutationLeaseTtl = opts.mutationLeaseTtl ?? 30_000; this.#stepRunner = createStepRunner(); } @@ -316,8 +311,7 @@ export class Reconciler { state: this.#state, dryRun, emit: this.#emit, - retryOptions: this.#retryOptions, - readPollOptions: this.#readPollOptions, + maxOperationAttempts: this.#maxOperationAttempts, expectedRev: stateNode?.rev ?? 0, }), ); @@ -332,8 +326,7 @@ export class Reconciler { patch: action.patch, dryRun, emit: this.#emit, - retryOptions: this.#retryOptions, - readPollOptions: this.#readPollOptions, + maxOperationAttempts: this.#maxOperationAttempts, expectedRev: stateNode!.rev, }), ); @@ -380,8 +373,7 @@ export class Reconciler { state: this.#state, dryRun, emit: this.#emit, - retryOptions: this.#retryOptions, - readPollOptions: this.#readPollOptions, + maxOperationAttempts: this.#maxOperationAttempts, expectedRev: stateNode?.rev ?? 0, }), ); @@ -395,8 +387,7 @@ export class Reconciler { patch: action.patch, dryRun, emit: this.#emit, - retryOptions: this.#retryOptions, - readPollOptions: this.#readPollOptions, + maxOperationAttempts: this.#maxOperationAttempts, expectedRev: stateNode?.rev ?? 0, }), ); @@ -431,20 +422,8 @@ export class Reconciler { let action = decideAction({ resource, stateNode, params }); if (action.decision === "noop" && driftDetection) { - try { - const driftRead = await this.#readForPlan(resource, params); - action = decideAction({ resource, stateNode, params, driftRead }); - } catch (error) { - if (!ResourceNotReadyError.is(error)) throw error; - return { - id: resource.id, - type: resource.type, - decision: "indeterminate", - reason: error.message, - params, - dependsOn: getDependencyIds(resource), - }; - } + const driftRead = await this.#readForDrift(resource); + action = decideAction({ resource, stateNode, params, driftRead }); } return { @@ -457,35 +436,21 @@ export class Reconciler { }; } - async #readForPlan( - resource: BaseResource, - params: Record, - ): Promise { - if (!resource.read) { - return { - kind: "present", - output: { ...resource.output, ...params }, - }; - } - - const output = await resource.read(resource.key); - return output === undefined - ? { kind: "absent" } - : { kind: "present", output: { ...params, ...output } }; - } - async #readForDrift(resource: BaseResource): Promise { - const output = await runOperation( - readResourceOperation(this.#stepRunner, { - resource, - state: this.#state, - emit: this.#emit, - readPollOptions: this.#readPollOptions, - }), - ); - return output === undefined - ? { kind: "absent" } - : { kind: "present", output }; + try { + const output = await runOperation( + readResourceOperation(this.#stepRunner, { + resource, + state: this.#state, + emit: this.#emit, + maxOperationAttempts: this.#maxOperationAttempts, + }), + ); + return { kind: "present", output }; + } catch (error) { + if (ResourceNotFoundError.is(error)) return { kind: "absent" }; + throw error; + } } async #deleteOrphans( @@ -575,7 +540,7 @@ export class Reconciler { state: this.#state, dryRun, emit: this.#emit, - retryOptions: this.#retryOptions, + maxOperationAttempts: this.#maxOperationAttempts, expectedRev: stateNode.rev, }), ); @@ -620,37 +585,7 @@ export function createStepRunner(): StepRunner { throw new Error("Missing run function"); } - while (true) { - try { - return await fn(); - } catch (err) { - if (!(err instanceof RetryableError)) { - throw err; - } - } - } - }, - async *poll( - arg1: string | PollOptions, - arg2: PollOptions | (() => boolean | Promise), - arg3?: () => boolean | Promise, - ): AsyncGenerator { - const opts = (typeof arg1 === "string" ? arg2 : arg1) as PollOptions; - const predicate = (typeof arg1 === "string" ? arg3 : arg2) as - (() => boolean | Promise) | undefined; - - if (!predicate) { - throw new Error("Missing poll predicate"); - } - - for (let attempt = 0; attempt < opts.maxAttempts; attempt += 1) { - if (await predicate()) return; - } - - throw new RetryableError("Polling reached max retries", { - maxAttempts: opts.maxAttempts, - retryInterval: opts.retryInterval, - }); + return await fn(); }, async *delay( arg1: string | number, diff --git a/packages/reconciler/test/operation.workflows.test.ts b/packages/reconciler/test/operation.workflows.test.ts index e6eb306..2afc6df 100644 --- a/packages/reconciler/test/operation.workflows.test.ts +++ b/packages/reconciler/test/operation.workflows.test.ts @@ -1,12 +1,14 @@ import { describe, expect, it, vi } from "vitest"; -import { RetryableError } from "yieldstar"; -import { resource, ResourceNotReadyError } from "@notation/resource"; +import { + resource, + ResourceNotFoundError, + ResourceOperationPendingError, +} from "@notation/resource"; import { createResourceOperation, deleteResourceOperation, readResourceOperation, type OperationLifecycleEvent, - type PollOptions, type StepRunner, } from "../src/operations"; @@ -20,40 +22,7 @@ function createStepRunnerDouble(): StepRunner { throw new Error("Missing run function"); } - while (true) { - try { - return await fn(); - } catch (err) { - if (!(err instanceof RetryableError)) { - throw err; - } - } - } - }); - - const poll = vi.fn(async function* ( - arg1: string | PollOptions, - arg2: PollOptions | (() => boolean | Promise), - arg3?: () => boolean | Promise, - ): AsyncGenerator { - const opts = (typeof arg1 === "string" ? arg2 : arg1) as PollOptions; - const predicate = (typeof arg1 === "string" ? arg3 : arg2) as - (() => boolean | Promise) | undefined; - - if (!predicate) { - throw new Error("Missing poll predicate"); - } - - for (let attempt = 0; attempt < opts.maxAttempts; attempt++) { - if (await predicate()) { - return; - } - } - - throw new RetryableError("Polling reached max retries", { - maxAttempts: opts.maxAttempts, - retryInterval: opts.retryInterval, - }); + return await fn(); }); const delay = vi.fn(async function* (): AsyncGenerator< @@ -66,7 +35,6 @@ function createStepRunnerDouble(): StepRunner { return { run, - poll, delay, }; } @@ -90,11 +58,17 @@ describe("operation workflows", () => { }; let createAttempts = 0; - const createMock = vi.fn(async () => { + const createMock = vi.fn(async (_params, context) => { createAttempts += 1; if (createAttempts === 1) { - throw new ResourceNotReadyError("retry create"); + expect(context).toBeUndefined(); + throw Object.assign(new Error("retry create"), { + _tag: "ResourceOperationPendingError", + retryAfterMs: 25, + callbackContext: { operationId: "create-123" }, + }); } + expect(context).toEqual({ operationId: "create-123" }); return { remoteId: "abc" }; }); @@ -121,7 +95,16 @@ describe("operation workflows", () => { expect(createAttempts).toBe(2); expect(state.update).toHaveBeenCalledOnce(); - expect(createMock).toHaveBeenCalledWith(await testResource.getParams()); + expect(createMock).toHaveBeenNthCalledWith( + 1, + await testResource.getParams(), + undefined, + ); + expect(createMock).toHaveBeenNthCalledWith( + 2, + await testResource.getParams(), + { operationId: "create-123" }, + ); expect(testResource.output).toEqual({ remoteId: "abc", status: "ready" }); expect(events.map((event) => `${event.operation}:${event.status}`)).toEqual( ["create:start", "read:start", "read:success", "create:success"], @@ -133,7 +116,7 @@ describe("operation workflows", () => { }); }); - it("read retries while the resource reports a not-ready condition", async () => { + it("read follows pending retry instructions", async () => { const step = createStepRunnerDouble(); const state = { get: vi.fn(async () => undefined), @@ -146,11 +129,15 @@ describe("operation workflows", () => { .defineSchema({}) .defineOperations({ create: async () => ({}), - read: async () => { + read: async (_key, context) => { readAttempts += 1; if (readAttempts < 3) { - throw new ResourceNotReadyError("resource is not ready"); + throw new ResourceOperationPendingError("resource is not ready", { + retryAfterMs: readAttempts * 10, + callbackContext: { readAttempts }, + }); } + expect(context).toEqual({ readAttempts: 2 }); return { status: "ready" } as const; }, delete: async () => undefined, @@ -167,38 +154,78 @@ describe("operation workflows", () => { expect(readAttempts).toBe(3); expect(result).toEqual({ status: "ready" }); + expect(step.delay).toHaveBeenNthCalledWith( + 1, + "read:remote:retry-delay:0", + 10, + ); + expect(step.delay).toHaveBeenNthCalledWith( + 2, + "read:remote:retry-delay:1", + 20, + ); }); - it("retries an absent read after creation until the resource is visible", async () => { + it("fails when an operation remains pending past the safety limit", async () => { + const step = createStepRunnerDouble(); + const state = { + get: vi.fn(async () => undefined), + update: vi.fn(async () => undefined), + delete: vi.fn(async () => undefined), + }; + const read = vi.fn(async () => { + throw new ResourceOperationPendingError("still pending", { + retryAfterMs: 10, + }); + }); + const TestResource = resource({ type: "test/service/pending-limit" }) + .defineSchema({}) + .defineOperations({ + create: async () => ({}), + read, + delete: async () => undefined, + }); + + await expect( + runOperation( + readResourceOperation(step, { + resource: new TestResource({ id: "pending-limit" }), + state, + maxOperationAttempts: 2, + }), + ), + ).rejects.toThrowError("still pending after 2 attempts"); + expect(read).toHaveBeenCalledTimes(2); + expect(step.delay).toHaveBeenCalledOnce(); + }); + + it("does not infer that not-found after creation is retryable", async () => { const step = createStepRunnerDouble(); const state = { get: vi.fn(async () => undefined), update: vi.fn(async () => undefined), delete: vi.fn(async () => undefined), }; - let readAttempts = 0; const TestResource = resource({ type: "test/service/eventually-visible" }) .defineSchema({}) .defineOperations({ create: async () => ({}), read: async () => { - readAttempts += 1; - if (readAttempts === 1) return undefined; - return { remoteId: "visible" } as const; + throw new ResourceNotFoundError("resource is absent"); }, delete: async () => undefined, }); - await runOperation( - createResourceOperation(step, { - resource: new TestResource({ id: "eventually-visible" }), - state, - expectedRev: 0, - }), - ); - - expect(readAttempts).toBe(2); - expect(state.update).toHaveBeenCalledOnce(); + await expect( + runOperation( + createResourceOperation(step, { + resource: new TestResource({ id: "eventually-visible" }), + state, + expectedRev: 0, + }), + ), + ).rejects.toThrowError("resource is absent"); + expect(state.update).not.toHaveBeenCalled(); }); it("delete treats an already-absent remote as success through its idempotent resource contract", async () => { diff --git a/packages/reconciler/test/reconciler.deploy.test.ts b/packages/reconciler/test/reconciler.deploy.test.ts index 2f6d359..eaa5ce5 100644 --- a/packages/reconciler/test/reconciler.deploy.test.ts +++ b/packages/reconciler/test/reconciler.deploy.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { LeaseConflict, MemoryStateBackend, @@ -59,9 +59,7 @@ function createTestResourceClass(opts: { create?: ( params: Record, ) => Promise | void>; - read?: ( - key: Record, - ) => Promise | undefined>; + read?: (key: Record) => Promise>; update?: ( key: Record, patch: Record, @@ -542,7 +540,9 @@ describe("reconciler destroy + refresh", () => { remoteExists = false; }); const readSpy = vi.fn(async () => { - if (!remoteExists) return undefined; + if (!remoteExists) { + throw new ResourceNotFoundError("resource is absent"); + } return found({ name: "doomed" }); }); const DestroyResource = createTestResourceClass({ diff --git a/packages/reconciler/test/reconciler.plan.test.ts b/packages/reconciler/test/reconciler.plan.test.ts index 9723899..d98e55a 100644 --- a/packages/reconciler/test/reconciler.plan.test.ts +++ b/packages/reconciler/test/reconciler.plan.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { - ResourceNotReadyError, + ResourceNotFoundError, + ResourceOperationPendingError, resource, type BaseResource, } from "@notation/resource"; @@ -41,9 +42,7 @@ function createTestResourceClass(opts: { create?: ( params: Record, ) => Promise | void>; - read?: ( - key: Record, - ) => Promise | undefined>; + read?: (key: Record) => Promise>; update?: ( key: Record, patch: Record, @@ -207,7 +206,9 @@ describe("reconciler plan", () => { it("plans drift-recreate when the remote resource is gone", async () => { const TestResource = createTestResourceClass({ type: "test/service/plan-drift-recreate", - read: async () => undefined, + read: async () => { + throw new ResourceNotFoundError("resource is absent"); + }, }); const state = createMemoryState({ @@ -229,11 +230,19 @@ describe("reconciler plan", () => { }); }); - it("plans indeterminate when the remote reports a not-ready condition", async () => { + it("waits for a pending read before planning", async () => { + let attempts = 0; const TestResource = createTestResourceClass({ type: "test/service/plan-not-ready", read: async () => { - throw new ResourceNotReadyError("Waiting for Lambda to become active"); + attempts += 1; + if (attempts === 1) { + throw new ResourceOperationPendingError( + "Waiting for Lambda to become active", + { retryAfterMs: 0 }, + ); + } + return { name: "desired" }; }, }); @@ -250,9 +259,9 @@ describe("reconciler plan", () => { expect(plan.nodes[0]).toMatchObject({ id: "resource", - decision: "indeterminate", - reason: "Waiting for Lambda to become active", + decision: "noop", }); + expect(attempts).toBe(2); }); it("skips remote reads when drift detection is off", async () => { diff --git a/packages/resource/src/index.ts b/packages/resource/src/index.ts index 46a899b..f531b96 100644 --- a/packages/resource/src/index.ts +++ b/packages/resource/src/index.ts @@ -1,6 +1,7 @@ export * from "./types"; export * from "./resource.schema"; export * from "./resource"; +export * from "./resource-operation"; export * from "./resource-group"; export { collectResourceGraph, diff --git a/packages/resource/src/resource-operation.ts b/packages/resource/src/resource-operation.ts new file mode 100644 index 0000000..87314a4 --- /dev/null +++ b/packages/resource/src/resource-operation.ts @@ -0,0 +1,60 @@ +export type ResourceOperationContext = Readonly>; + +export class ResourceNotFoundError extends Error { + readonly _tag = "ResourceNotFoundError" as const; + + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = "ResourceNotFoundError"; + } + + static is(error: unknown): error is ResourceNotFoundError { + return hasTag(error, "ResourceNotFoundError"); + } +} + +export class ResourceOperationPendingError extends Error { + readonly _tag = "ResourceOperationPendingError" as const; + readonly retryAfterMs: number; + readonly callbackContext?: ResourceOperationContext; + + constructor( + message: string, + options: ErrorOptions & { + retryAfterMs: number; + callbackContext?: ResourceOperationContext; + }, + ) { + super(message, options); + this.name = "ResourceOperationPendingError"; + this.retryAfterMs = options.retryAfterMs; + this.callbackContext = options.callbackContext; + + if (!Number.isFinite(options.retryAfterMs) || options.retryAfterMs < 0) { + throw new RangeError("retryAfterMs must be a non-negative number"); + } + } + + static is(error: unknown): error is ResourceOperationPendingError { + return ( + hasTag(error, "ResourceOperationPendingError") && + "retryAfterMs" in error && + typeof error.retryAfterMs === "number" + ); + } +} + +export type ResourceOperationSignal = + ResourceNotFoundError | ResourceOperationPendingError; + +function hasTag( + error: unknown, + tag: T, +): error is Error & { _tag: T } { + return ( + typeof error === "object" && + error !== null && + "_tag" in error && + error._tag === tag + ); +} diff --git a/packages/resource/src/resource.ts b/packages/resource/src/resource.ts index 0b42402..f191059 100644 --- a/packages/resource/src/resource.ts +++ b/packages/resource/src/resource.ts @@ -17,37 +17,12 @@ import type { Fallback, NoInfer, } from "./types"; +import type { ResourceOperationContext } from "./resource-operation"; export type { Schema, SchemaItem, DefineResourceApiSchema }; export type ResourceType = `${string}/${string}/${string}`; -/** - * Thrown by a resource operation when the provider reports a known temporary - * condition — the resource exists but is not yet usable, or a dependency has - * not finished propagating. - * - * Consumers recognise it by its declared `_tag`, not by class identity, so it - * survives being thrown across package or realm boundaries. - */ -export class ResourceNotReadyError extends Error { - readonly _tag = "ResourceNotReadyError"; - - constructor(message: string, options?: ErrorOptions) { - super(message, options); - this.name = "ResourceNotReadyError"; - } - - static is(error: unknown): error is ResourceNotReadyError { - return ( - typeof error === "object" && - error !== null && - "_tag" in error && - (error as { _tag: unknown })._tag === "ResourceNotReadyError" - ); - } -} - export type ResourceOpts = OptionalIfAllPropertiesOptional<"config", C> & OptionalIfAllPropertiesOptional<"dependencies", D> & { id: string }; @@ -88,10 +63,26 @@ export interface BaseResource { readonly output: {}; readonly dependencies: Record; readonly key: {}; - create: (params: any) => Promise<{} | void>; - read?: (key: any) => Promise | undefined>; - update?: (key: any, patch: any, params: any, state: any) => Promise; - delete: (key: any, state: any) => Promise; + create: ( + params: any, + context?: ResourceOperationContext, + ) => Promise<{} | void>; + read?: ( + key: any, + context?: ResourceOperationContext, + ) => Promise>; + update?: ( + key: any, + patch: any, + params: any, + state: any, + context?: ResourceOperationContext, + ) => Promise; + delete: ( + key: any, + state: any, + context?: ResourceOperationContext, + ) => Promise; getParams(): Promise<{}>; toState(output: {}): {}; toComparable(output: {}): {}; @@ -116,15 +107,26 @@ export abstract class Resource< dependencies = {} as NoInfer; abstract type: ResourceType; abstract schema: Schema; - abstract create: (params: T["params"]) => Promise; - abstract read?: (key: T["compoundKey"]) => Promise; + abstract create: ( + params: T["params"], + context?: ResourceOperationContext, + ) => Promise; + abstract read?: ( + key: T["compoundKey"], + context?: ResourceOperationContext, + ) => Promise; abstract update?: ( key: T["compoundKey"], patch: T["params"], params: T["params"], state: T["state"], + context?: ResourceOperationContext, + ) => Promise; + abstract delete: ( + key: T["compoundKey"], + state: T["state"], + context?: ResourceOperationContext, ) => Promise; - abstract delete: (key: T["compoundKey"], state: T["state"]) => Promise; abstract deriveParams(opts: { id: string; config: C; @@ -196,15 +198,26 @@ export type ResourceOperationsOptions< T extends ResourceTypes, IntrinsicParams extends Partial, > = { - create: (params: T["params"]) => Promise; - read?: (key: T["compoundKey"]) => Promise; + create: ( + params: T["params"], + context?: ResourceOperationContext, + ) => Promise; + read?: ( + key: T["compoundKey"], + context?: ResourceOperationContext, + ) => Promise; update?: ( key: T["compoundKey"], patch: T["params"], params: T["params"], state: T["state"], + context?: ResourceOperationContext, + ) => Promise; + delete: ( + key: T["compoundKey"], + state: T["state"], + context?: ResourceOperationContext, ) => Promise; - delete: (key: T["compoundKey"], state: T["state"]) => Promise; deriveParams?: (opts: { config: Partial; }) => IntrinsicParams | Promise; diff --git a/packages/resource/test/resource.test.ts b/packages/resource/test/resource.test.ts index 8b2163f..b3e3b7e 100644 --- a/packages/resource/test/resource.test.ts +++ b/packages/resource/test/resource.test.ts @@ -1,5 +1,9 @@ import { expect, it, test, vi } from "vitest"; -import { ResourceNotReadyError, resource } from "src"; +import { + ResourceNotFoundError, + ResourceOperationPendingError, + resource, +} from "src"; import { TestResource, testResourceConfig, @@ -139,24 +143,51 @@ describe("resource dependencies", () => { }); }); -describe("ResourceNotReadyError", () => { - it("recognises its own instances", () => { - expect(ResourceNotReadyError.is(new ResourceNotReadyError("waiting"))).toBe( +describe("resource operation signals", () => { + it("recognises a not-found signal structurally", () => { + expect(ResourceNotFoundError.is(new ResourceNotFoundError("missing"))).toBe( true, ); + expect( + ResourceNotFoundError.is( + Object.assign(new Error("missing"), { + _tag: "ResourceNotFoundError", + }), + ), + ).toBe(true); }); - it("recognises the tag without class identity", () => { - // A copy thrown from another realm or bundle carries the tag, not the class. + it("recognises a pending signal structurally", () => { const fromElsewhere = Object.assign(new Error("waiting"), { - _tag: "ResourceNotReadyError", + _tag: "ResourceOperationPendingError", + retryAfterMs: 2_000, + callbackContext: { operationId: "abc" }, }); - expect(ResourceNotReadyError.is(fromElsewhere)).toBe(true); + expect(ResourceOperationPendingError.is(fromElsewhere)).toBe(true); }); it("rejects unrelated errors", () => { - expect(ResourceNotReadyError.is(new Error("boom"))).toBe(false); - expect(ResourceNotReadyError.is({ _tag: "SomethingElse" })).toBe(false); - expect(ResourceNotReadyError.is(undefined)).toBe(false); + expect(ResourceNotFoundError.is(new Error("boom"))).toBe(false); + expect(ResourceOperationPendingError.is(new Error("boom"))).toBe(false); + expect( + ResourceOperationPendingError.is({ + _tag: "ResourceOperationPendingError", + }), + ).toBe(false); + }); + + it("carries retry instructions and callback context", () => { + const pending = new ResourceOperationPendingError("waiting", { + retryAfterMs: 1_500, + callbackContext: { operationId: "abc" }, + }); + expect(pending.retryAfterMs).toBe(1_500); + expect(pending.callbackContext).toEqual({ operationId: "abc" }); + }); + + it("rejects an invalid retry delay", () => { + expect( + () => new ResourceOperationPendingError("waiting", { retryAfterMs: -1 }), + ).toThrowError("retryAfterMs must be a non-negative number"); }); }); diff --git a/packages/std.iac/src/resources/fs/file.ts b/packages/std.iac/src/resources/fs/file.ts index 5cc42fc..86c32cc 100644 --- a/packages/std.iac/src/resources/fs/file.ts +++ b/packages/std.iac/src/resources/fs/file.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { isErrorWithCode } from "@notation/utils"; import { getSourceSha256 } from "src/utils/hash"; import * as fs from "node:fs/promises"; @@ -42,7 +42,7 @@ export const File = fileSchema.defineOperations({ return { ...config, file }; } catch (error) { if (isErrorWithCode(error, "ENOENT")) { - return undefined; + throw new ResourceNotFoundError("File was not found", { cause: error }); } throw error; } diff --git a/packages/std.iac/src/resources/fs/zip.ts b/packages/std.iac/src/resources/fs/zip.ts index 031d547..dae22d6 100644 --- a/packages/std.iac/src/resources/fs/zip.ts +++ b/packages/std.iac/src/resources/fs/zip.ts @@ -1,4 +1,4 @@ -import { resource } from "@notation/resource"; +import { resource, ResourceNotFoundError } from "@notation/resource"; import { isErrorWithCode } from "@notation/utils"; import * as fs from "node:fs/promises"; import { zip } from "src/utils/zip"; @@ -57,7 +57,9 @@ export const Zip = zipSchema.defineOperations({ return { ...params, file }; } catch (error) { if (isErrorWithCode(error, "ENOENT")) { - return undefined; + throw new ResourceNotFoundError("Zip archive was not found", { + cause: error, + }); } throw error; } From 266bafc4a6f45373c6901ad540812b7bf32d694c Mon Sep 17 00:00:00 2001 From: djgrant <1670902+djgrant@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:02:58 +0100 Subject: [PATCH 6/6] Document the resource error API --- docs/internals/reconciler.md | 13 ++++- docs/internals/resource.md | 64 ++++++++++++++++++++- packages/resource/src/resource-operation.ts | 14 +++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/docs/internals/reconciler.md b/docs/internals/reconciler.md index e37b4f5..6b0d413 100644 --- a/docs/internals/reconciler.md +++ b/docs/internals/reconciler.md @@ -83,9 +83,16 @@ Each CRUD operation is implemented as an async generator with retry support: ### Pending operations -A resource operation reports that it is still in progress by throwing `ResourceOperationPendingError` with a retry delay and optional callback context. The reconciler persists that context, waits for the requested delay, and invokes the operation again. It does not infer retry behaviour from provider errors. +A resource operation throws `ResourceOperationPendingError` when it has not finished. The reconciler reads two fields from the error: -The reconciler limits the number of attempts as a safety boundary: +| Field | Action | +| ----- | ------ | +| `retryAfterMs` | Wait this many milliseconds. | +| `callbackContext` | Pass this value to the next call of the same operation. | + +The reconciler then calls the same operation again. Any other error fails the operation. See [Operation errors](./resource.md#operation-errors) for the complete API. + +The default limit is 30 calls to one operation: ```ts [packages/reconciler/src/index.ts] { @@ -93,7 +100,7 @@ The reconciler limits the number of attempts as a safety boundary: } ``` -Resources provide the timing because they understand the remote operation. For example, the Lambda resource reports IAM propagation and inactive function states as pending. +The last pending error becomes a failure when the limit is reached. ### Operation lifecycle diff --git a/docs/internals/resource.md b/docs/internals/resource.md index fc0a61c..baae261 100644 --- a/docs/internals/resource.md +++ b/docs/internals/resource.md @@ -147,11 +147,69 @@ All schema items carry these fields: | `delete` | yes | `(key, state, context?) => Promise` – ensure the resource is absent. Implementations must also succeed when the remote resource is already gone. | | `deriveParams` | no | Computes intrinsic derived params from config (not dependency-aware). | -Resource operations translate provider-specific responses at the provider boundary. A read throws the structurally tagged `ResourceNotFoundError` when the provider says the resource does not exist. Delete implementations consume the equivalent provider error and complete successfully. +### Operation errors -An operation that has started but not settled throws `ResourceOperationPendingError`. The error supplies the delay before the next attempt and may supply callback context for that attempt. The operation decides when this is appropriate: for example, a create handler may treat temporary absence as pending while an ordinary read reports not-found. +Import the errors from `@notation/resource`: -The reconciler does not infer behaviour from provider error names, messages, or the operation being run. It persists callback context, waits for the requested delay, and invokes the same operation again. A configured maximum attempt count remains a framework safety limit. Every error other than the two declared resource-operation signals fails the operation. +```ts +import { + ResourceNotFoundError, + ResourceOperationPendingError, +} from "@notation/resource"; +``` + +The constructors are: + +```ts +new ResourceNotFoundError(message: string, options?: { cause?: unknown }); + +new ResourceOperationPendingError(message: string, { + retryAfterMs: number; + callbackContext?: Readonly>; + cause?: unknown; +}); +``` + +| Handler result | Meaning | What the reconciler does | +| -------------- | ------- | ------------------------ | +| Return normally | The operation finished. | Continues the deployment. | +| `throw new ResourceNotFoundError(message, { cause })` | `read` found no resource for the given key. | Treats the resource as absent during planning and refresh. A read after create or update fails because that operation claimed to have finished. | +| `throw new ResourceOperationPendingError(message, { retryAfterMs, callbackContext })` | The operation has not finished. | Waits for `retryAfterMs`, then calls the same handler again. It passes `callbackContext` as the handler's final argument. | +| Throw any other error | The operation failed. | Stops the deployment. | + +`ResourceNotFoundError` is for `read`. A `delete` handler must catch the provider's missing-resource error and return normally. + +`ResourceOperationPendingError` may be thrown by `create`, `read`, `update`, or `delete`. Its options are: + +| Option | Type | Required | Meaning | +| ------ | ---- | -------- | ------- | +| `retryAfterMs` | `number` | yes | Milliseconds to wait. It must be zero or greater. | +| `callbackContext` | `Readonly>` | no | Plain serializable data for the next attempt. | +| `cause` | `unknown` | no | The provider error that caused this result. | + +The default limit is 30 attempts. Set `maxOperationAttempts` on the reconciler to change it. Reaching the limit fails the operation. + +```ts +read: async (key, context) => { + try { + return await client.send(new GetResourceCommand(key)); + } catch (error) { + if (error instanceof ResourceMissingException) { + throw new ResourceNotFoundError("Resource was not found", { + cause: error, + }); + } + if (error instanceof OperationInProgressException) { + throw new ResourceOperationPendingError("Resource is not ready", { + retryAfterMs: 1_000, + callbackContext: { requestId: error.requestId }, + cause: error, + }); + } + throw error; + } +}; +``` ## Dependencies diff --git a/packages/resource/src/resource-operation.ts b/packages/resource/src/resource-operation.ts index 87314a4..897dea4 100644 --- a/packages/resource/src/resource-operation.ts +++ b/packages/resource/src/resource-operation.ts @@ -1,5 +1,9 @@ +/** Plain data passed from one attempt of an operation to the next. */ export type ResourceOperationContext = Readonly>; +/** + * A `read` operation throws this error when no resource exists for its key. + */ export class ResourceNotFoundError extends Error { readonly _tag = "ResourceNotFoundError" as const; @@ -13,9 +17,19 @@ export class ResourceNotFoundError extends Error { } } +/** + * An operation throws this error when it has not finished. + * + * The reconciler waits for `retryAfterMs`, then calls the same operation again + * with `callbackContext`. + */ export class ResourceOperationPendingError extends Error { readonly _tag = "ResourceOperationPendingError" as const; + + /** Milliseconds to wait before the next attempt. */ readonly retryAfterMs: number; + + /** Plain data passed to the next attempt of the same operation. */ readonly callbackContext?: ResourceOperationContext; constructor(