fix: don't fire gated automations when their data source errors

+22 -3

docs/data-source-fetches.md

··· 213 213 "skipped by <fetchName>" log so authors can debug why the automation isn't 214 214 firing. 215 215 216 + A fetch that errors while carrying conditions is also treated as a skip; 217 + see [Interaction with fetch errors](#interaction-with-fetch-errors). 218 + 216 219 ## Interaction with fetch errors 217 220 218 221 A fetch **error** (bad URI, PDS unreachable, search throws) is distinct from a 219 222 fetch **not-finding** anything. Errors are collected into the `errors` array 220 - on the `FetchResolution`; the entry is not added to the context, subsequent 221 - fetches that template against it get `undefined`. Dry-run surfaces these as 222 - "Fetch failed: <name>" entries; real runs log them to console but continue. 223 + on the `FetchResolution` and the entry is not added to the context. 224 + 225 + Whether the automation fires when a fetch errors depends on whether that 226 + fetch carries **conditions**: 227 + 228 + - **Errored fetch with conditions** → `resolveFetches` returns 229 + `skip: true`, `skippedBy: <step.name>`, `skipCausedByError: true`. The 230 + automation does not fire. Rationale: a condition is the user declaring 231 + "only fire if this holds." A transport error means we cannot prove it 232 + holds, so the safe default for a gated fetch is to not fire. Dry-run 233 + writes `Skipped: data source "<name>" errored and had conditions` so the 234 + author can see why. 235 + - **Errored fetch without conditions** (enrichment-only) → the error is 236 + logged but resolution continues. Subsequent fetches that template against 237 + the missing entry will see `undefined`. Dry-run still surfaces the error 238 + per action as `Fetch failed: <name>`. 239 + 240 + Real runs always log the underlying error to the console regardless of the 241 + skip path. 223 242 224 243 The typical pattern for "only act if X doesn't exist yet" therefore looks like: 225 244

+47 -3

lib/actions/fetcher.test.ts

··· 6 6 fetchRecord: vi.fn(), 7 7 })); 8 8 9 + vi.mock("./searcher.js", () => ({ 10 + executeSearch: vi.fn(), 11 + })); 12 + 9 13 import { fetchRecord } from "../pds/resolver.js"; 14 + import { executeSearch } from "./searcher.js"; 10 15 const mockFetchRecord = vi.mocked(fetchRecord); 16 + const mockExecuteSearch = vi.mocked(executeSearch); 11 17 12 18 describe("resolveFetches", () => { 13 19 const ownerDid = "did:plc:owner"; ··· 15 21 16 22 beforeEach(() => { 17 23 mockFetchRecord.mockReset(); 24 + mockExecuteSearch.mockReset(); 18 25 }); 19 26 20 27 it("resolves a single fetch step", async () => { ··· 249 256 expect(result.context.search1).toBeUndefined(); 250 257 }); 251 258 252 - it("does not evaluate conditions on a record step that errored", async () => { 259 + it("skips when an errored record fetch had conditions attached", async () => { 253 260 mockFetchRecord.mockRejectedValueOnce(new Error("PDS unreachable")); 254 261 255 262 const result = await resolveFetches( ··· 257 264 { 258 265 name: "failing", 259 266 uri: "at://did1/col/rk1", 260 - // Even with a condition that would fail on a missing entry, 261 - // the error path takes precedence — no skip is emitted. 262 267 conditions: [{ field: "found", operator: "exists", value: "" }], 263 268 }, 264 269 ], ··· 266 271 ownerDid, 267 272 ); 268 273 274 + expect(result.skip).toBe(true); 275 + expect(result.skippedBy).toBe("failing"); 276 + expect(result.skipCausedByError).toBe(true); 277 + expect(result.errors).toHaveLength(1); 278 + }); 279 + 280 + it("continues when an errored record fetch has no conditions (enrichment-only)", async () => { 281 + mockFetchRecord.mockRejectedValueOnce(new Error("PDS unreachable")); 282 + 283 + const result = await resolveFetches( 284 + [{ name: "failing", uri: "at://did1/col/rk1" }], 285 + event, 286 + ownerDid, 287 + ); 288 + 269 289 expect(result.skip).toBe(false); 290 + expect(result.errors).toHaveLength(1); 291 + }); 292 + 293 + it("skips when an errored search fetch had conditions attached", async () => { 294 + mockExecuteSearch.mockRejectedValueOnce(new Error("listRecords timeout")); 295 + 296 + const result = await resolveFetches( 297 + [ 298 + { 299 + kind: "search", 300 + name: "existingMirror", 301 + repo: "{{self}}", 302 + collection: "id.sifa.graph.follow", 303 + where: [{ field: "subject", operator: "eq", value: "x" }], 304 + conditions: [{ field: "found", operator: "not-exists", value: "" }], 305 + }, 306 + ], 307 + event, 308 + ownerDid, 309 + ); 310 + 311 + expect(result.skip).toBe(true); 312 + expect(result.skippedBy).toBe("existingMirror"); 313 + expect(result.skipCausedByError).toBe(true); 270 314 expect(result.errors).toHaveLength(1); 271 315 }); 272 316 });

+19 -4

lib/actions/fetcher.ts

··· 28 28 export type FetchResolution = { 29 29 context: FetchContext; 30 30 errors: Array<{ name: string; error: string }>; 31 - /** True when a per-fetch condition failed. Handler bails before actions. */ 31 + /** True when a per-fetch condition failed, or when a fetch with conditions 32 + * errored (we can't prove the gate holds, so don't fire). */ 32 33 skip: boolean; 33 - /** Name of the fetch whose conditions triggered the skip, set alongside 34 - * `skip: true`. Used by dry-run to write an informative delivery log. */ 34 + /** Name of the fetch that triggered the skip, set alongside `skip: true`. 35 + * Used by dry-run to write an informative delivery log. */ 35 36 skippedBy?: string; 37 + /** True when the skip was caused by a fetch error rather than a condition 38 + * evaluating to false. Lets dry-run surface a distinct reason. */ 39 + skipCausedByError?: boolean; 36 40 }; 37 41 38 42 /** Resolve all fetch steps, returning available context, any errors, and ··· 74 78 // If any fail, short-circuit before running search steps. 75 79 for (const step of recordSteps) { 76 80 const entry = context[step.name]; 77 - if (!entry) continue; // error path — don't attempt condition gate 81 + if (!entry) { 82 + // Error path: if the step declared conditions, treat the error as a 83 + // gate failure rather than silently proceeding. Enrichment-only 84 + // fetches (no conditions) keep the log-and-continue behavior. 85 + if (step.conditions && step.conditions.length > 0) { 86 + return { context, errors, skip: true, skippedBy: step.name, skipCausedByError: true }; 87 + } 88 + continue; 89 + } 78 90 if (!evaluateFetchConditions(entry, step.conditions, ownerDid)) { 79 91 return { context, errors, skip: true, skippedBy: step.name }; 80 92 } ··· 92 104 name: step.name, 93 105 error: err instanceof Error ? err.message : String(err), 94 106 }); 107 + if (step.conditions && step.conditions.length > 0) { 108 + return { context, errors, skip: true, skippedBy: step.name, skipCausedByError: true }; 109 + } 95 110 } 96 111 } 97 112

+31

lib/jetstream/handler.test.ts

··· 215 215 ); 216 216 }); 217 217 218 + it("dry-run delivery log distinguishes a skip caused by a fetch error", async () => { 219 + mockResolveFetches.mockResolvedValueOnce({ 220 + context: {}, 221 + errors: [{ name: "existingMirror", error: "PDS unreachable" }], 222 + skip: true, 223 + skippedBy: "existingMirror", 224 + skipCausedByError: true, 225 + }); 226 + mockInsert.mockClear(); 227 + mockInsertValues.mockClear(); 228 + 229 + const match = makeMatch({ 230 + automation: { 231 + actions: [makeWebhookAction()], 232 + fetches: [makeFetchStep({ name: "existingMirror" })], 233 + dryRun: true, 234 + }, 235 + }); 236 + 237 + await handleMatchedEvent(match); 238 + 239 + expect(mockDispatch).not.toHaveBeenCalled(); 240 + expect(mockInsertValues).toHaveBeenCalledTimes(1); 241 + expect(mockInsertValues).toHaveBeenCalledWith( 242 + expect.objectContaining({ 243 + dryRun: true, 244 + message: expect.stringContaining("errored and had conditions"), 245 + }), 246 + ); 247 + }); 248 + 218 249 it("dry-run follow log advertises the built-in safety checks", async () => { 219 250 // The real `executeFollow` runs profile + already-follows pre-flight checks 220 251 // inside the action handler. Dry-run skips those (would burn PDS calls for

+9 -3

lib/jetstream/handler.ts

··· 46 46 // debug why their automation isn't firing. 47 47 if (result.skip) { 48 48 if (match.automation.dryRun) { 49 - await logDrySkip(match, result.skippedBy); 49 + await logDrySkip(match, result.skippedBy, result.skipCausedByError ?? false); 50 50 } 51 51 return; 52 52 } ··· 266 266 notifyAutomationChange(); 267 267 } 268 268 269 - async function logDrySkip(match: MatchedEvent, skippedBy: string | undefined) { 269 + async function logDrySkip( 270 + match: MatchedEvent, 271 + skippedBy: string | undefined, 272 + causedByError: boolean, 273 + ) { 270 274 const message = skippedBy 271 - ? `Skipped: conditions on data source "${skippedBy}" did not match` 275 + ? causedByError 276 + ? `Skipped: data source "${skippedBy}" errored and had conditions` 277 + : `Skipped: conditions on data source "${skippedBy}" did not match` 272 278 : "Skipped: per-source conditions did not match"; 273 279 await db.insert(deliveryLogs).values({ 274 280 automationUri: match.automation.uri,

Configure Feed

Configure Feed