Skip to content

Commit b7940da

Browse files
os-zhuangclaude
andauthored
fix(seed-loader): report deferred reference back-fill failures instead of swallowing them (#2805)
A circular reference between two seed records is resolved in two passes: the parent is inserted first without the back-reference (deferred), then pass 2 issues an engine.update to fill it in. If that pass-2 update threw, resolveDeferredUpdates only logged a warning — the reference stayed NULL yet the load still returned success: true, errors: [], totalErrored: 0. An incomplete relationship was reported as a clean load. Route both pass-2 failure modes (target still missing, or the back-fill write threw) through a single recordDeferredError helper that pushes a ReferenceResolutionError into the object's per-result errors, bumps its errored count so summary.totalErrored is truthful, and joins allErrors so the load reports success: false — matching how pass 1's update path already reports write failures. Note: a lone transient blip on the back-fill still recovers via withTransientRetry (covered by a test); only a failure that outlasts the retry budget — or a non-transient error — is reported. Claude-Session: https://claude.ai/code/session_01MAg2Tzkv69j9zx6ZyyrbHC Co-authored-by: Claude <noreply@anthropic.com>
1 parent a9459e6 commit b7940da

2 files changed

Lines changed: 235 additions & 18 deletions

File tree

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi } from 'vitest';
4+
import { SeedLoaderService } from './seed-loader';
5+
import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts';
6+
7+
/**
8+
* framework#2805: a pass-2 (deferred) reference back-fill that FAILS must be
9+
* reported — not silently swallowed.
10+
*
11+
* Two records reference each other (a circular dependency). The parent is
12+
* inserted first without the back-reference (deferred to pass 2); pass 2 then
13+
* issues an `engine.update` to fill the reference in. Before this fix, if that
14+
* update threw, `resolveDeferredUpdates` only logged a warning: the reference
15+
* stayed NULL yet the loader still returned `success: true`, `errors: []`,
16+
* `totalErrored: 0` — an incomplete relationship reported as a clean load.
17+
*/
18+
19+
function createLogger() {
20+
return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() };
21+
}
22+
23+
function createFaithfulEngine(): { engine: IDataEngine; store: Record<string, any[]> } {
24+
const store: Record<string, any[]> = {};
25+
let idCounter = 0;
26+
27+
const engine = {
28+
find: vi.fn(async (objectName: string, query?: any) => {
29+
let records = store[objectName] || [];
30+
if (query?.where) {
31+
records = records.filter((r) =>
32+
Object.entries(query.where).every(([k, v]) => r[k] === v),
33+
);
34+
}
35+
if (typeof query?.limit === 'number') records = records.slice(0, query.limit);
36+
return records;
37+
}),
38+
findOne: vi.fn(async (objectName: string, query?: any) => {
39+
const rows = await (engine.find as any)(objectName, { ...query, limit: 1 });
40+
return rows[0] ?? null;
41+
}),
42+
insert: vi.fn(async (objectName: string, data: any) => {
43+
if (!store[objectName]) store[objectName] = [];
44+
if (Array.isArray(data)) {
45+
const records = data.map((d) => ({ id: `gen-${++idCounter}`, ...d }));
46+
store[objectName].push(...records);
47+
return records;
48+
}
49+
const record = { id: `gen-${++idCounter}`, ...data };
50+
store[objectName].push(record);
51+
return record;
52+
}),
53+
update: vi.fn(async (objectName: string, data: any) => {
54+
const records = store[objectName] || [];
55+
const idx = records.findIndex((r) => r.id === data.id);
56+
if (idx >= 0) { records[idx] = { ...records[idx], ...data }; return records[idx]; }
57+
return data;
58+
}),
59+
delete: vi.fn(async () => ({ deleted: 1 })),
60+
count: vi.fn(async (objectName: string) => (store[objectName] || []).length),
61+
aggregate: vi.fn(async () => []),
62+
} as unknown as IDataEngine;
63+
64+
return { engine, store };
65+
}
66+
67+
// Two objects that reference each other → a circular dependency that forces
68+
// the multi-pass deferred back-fill (audit_department.head_id is filled in
69+
// during pass 2, once audit_worker "Alice" exists).
70+
function createMetadata(): IMetadataService {
71+
const objects: Record<string, any> = {
72+
audit_department: {
73+
name: 'audit_department',
74+
fields: {
75+
name: { type: 'text' },
76+
head_id: { type: 'lookup', reference: 'audit_worker' },
77+
},
78+
},
79+
audit_worker: {
80+
name: 'audit_worker',
81+
fields: {
82+
name: { type: 'text' },
83+
department_id: { type: 'lookup', reference: 'audit_department' },
84+
},
85+
},
86+
};
87+
return {
88+
getObject: vi.fn(async (name: string) => objects[name]),
89+
listObjects: vi.fn(async () => Object.values(objects)),
90+
register: vi.fn(async () => {}),
91+
get: vi.fn(async (_t: string, name: string) => objects[name]),
92+
list: vi.fn(async () => []),
93+
unregister: vi.fn(async () => {}),
94+
exists: vi.fn(async () => false),
95+
listNames: vi.fn(async () => []),
96+
} as unknown as IMetadataService;
97+
}
98+
99+
const CONFIG = {
100+
dryRun: false,
101+
haltOnError: false,
102+
multiPass: true,
103+
defaultMode: 'insert',
104+
batchSize: 1000,
105+
transaction: false,
106+
} as any;
107+
108+
const SEEDS = [
109+
{
110+
object: 'audit_department',
111+
externalId: 'name',
112+
mode: 'insert',
113+
env: ['prod', 'dev', 'test'],
114+
records: [{ name: 'Engineering', head_id: 'Alice' }],
115+
},
116+
{
117+
object: 'audit_worker',
118+
externalId: 'name',
119+
mode: 'insert',
120+
env: ['prod', 'dev', 'test'],
121+
records: [{ name: 'Alice', department_id: 'Engineering' }],
122+
},
123+
] as any[];
124+
125+
describe('seed deferred back-fill failure is reported, not swallowed (framework#2805)', () => {
126+
it('a failing pass-2 reference update flips success=false and counts an error', async () => {
127+
const { engine, store } = createFaithfulEngine();
128+
const metadata = createMetadata();
129+
130+
// The ONLY update in this load is pass-2's back-fill of
131+
// audit_department.head_id. Make every attempt fail (a persistent
132+
// "fetch failed" outlasts the transient-retry budget) so the deferred
133+
// reference genuinely never lands.
134+
const realUpdate = (engine.update as any).getMockImplementation();
135+
let deptUpdateAttempts = 0;
136+
(engine.update as any).mockImplementation(async (obj: string, data: any, opts: any) => {
137+
if (obj === 'audit_department') {
138+
deptUpdateAttempts++;
139+
throw new Error('fetch failed');
140+
}
141+
return realUpdate(obj, data, opts);
142+
});
143+
144+
const result = await new SeedLoaderService(engine, metadata, createLogger()).load({
145+
seeds: SEEDS,
146+
config: CONFIG,
147+
});
148+
149+
// The back-fill was attempted (and exhausted its retries).
150+
expect(deptUpdateAttempts).toBeGreaterThan(0);
151+
152+
// The relationship is genuinely incomplete: Engineering.head_id is still null.
153+
const engineeringRow = store.audit_department.find((r) => r.name === 'Engineering')!;
154+
expect(engineeringRow.head_id == null).toBe(true);
155+
156+
// ...so the load must NOT report clean success.
157+
expect(result.success).toBe(false);
158+
expect(result.summary.totalErrored).toBeGreaterThan(0);
159+
expect(result.errors.length).toBeGreaterThan(0);
160+
expect(result.errors.some((e: { field: string }) => e.field === 'head_id')).toBe(true);
161+
});
162+
163+
it('a transient blip that recovers on retry still reports clean success', async () => {
164+
const { engine, store } = createFaithfulEngine();
165+
const metadata = createMetadata();
166+
167+
// First back-fill attempt blips, the retry succeeds — the reference lands,
168+
// so this is NOT an error.
169+
const realUpdate = (engine.update as any).getMockImplementation();
170+
let deptUpdateAttempts = 0;
171+
(engine.update as any).mockImplementation(async (obj: string, data: any, opts: any) => {
172+
if (obj === 'audit_department') {
173+
deptUpdateAttempts++;
174+
if (deptUpdateAttempts === 1) throw new Error('fetch failed');
175+
}
176+
return realUpdate(obj, data, opts);
177+
});
178+
179+
const result = await new SeedLoaderService(engine, metadata, createLogger()).load({
180+
seeds: SEEDS,
181+
config: CONFIG,
182+
});
183+
184+
expect(deptUpdateAttempts).toBe(2); // blipped once, then succeeded
185+
const aliceId = store.audit_worker.find((r) => r.name === 'Alice')!.id;
186+
expect(store.audit_department.find((r) => r.name === 'Engineering')!.head_id).toBe(aliceId);
187+
expect(result.success).toBe(true);
188+
expect(result.summary.totalErrored).toBe(0);
189+
});
190+
});

packages/metadata-protocol/src/seed-loader.ts

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -770,34 +770,61 @@ export class SeedLoaderService implements ISeedLoaderService {
770770
resultEntry.referencesDeferred--;
771771
}
772772
} catch (err: any) {
773-
this.logger.warn('[SeedLoader] Failed to resolve deferred reference', {
773+
// LOUD FAILURE (framework#2805): the target resolved but the
774+
// back-fill WRITE failed (a transient error that outlasted the
775+
// retry budget, a validation veto, …). The reference stays NULL —
776+
// the very corruption pass 2 exists to prevent — so this must be a
777+
// reported, counted error, never a silent warning. Swallowing it
778+
// returned `success: true` / `totalErrored: 0` over a load that
779+
// left a circular relationship half-written.
780+
this.logger.warn('[SeedLoader] Failed to write deferred reference', {
774781
object: deferred.objectName,
775782
field: deferred.field,
776-
error: err.message,
783+
error: err?.message,
777784
});
785+
this.recordDeferredError(deferred, allResults, allErrors,
786+
`Failed to write deferred reference: ${deferred.objectName}.${deferred.field} = '${deferred.attemptedValue}' → ${deferred.targetObject}.${deferred.targetField}: ${err?.message ?? String(err)}`);
778787
}
779788
}
780789
} else {
781-
// Still unresolved after pass 2
782-
const error: ReferenceResolutionError = {
783-
sourceObject: deferred.objectName,
784-
field: deferred.field,
785-
targetObject: deferred.targetObject,
786-
targetField: deferred.targetField,
787-
attemptedValue: deferred.attemptedValue,
788-
recordIndex: deferred.recordIndex,
789-
message: `Deferred reference unresolved after pass 2: ${deferred.objectName}.${deferred.field} = '${deferred.attemptedValue}' → ${deferred.targetObject}.${deferred.targetField} not found`,
790-
};
791-
792-
const resultEntry = allResults.find(r => r.object === deferred.objectName);
793-
if (resultEntry) {
794-
resultEntry.errors.push(error);
795-
}
796-
allErrors.push(error);
790+
// Still unresolved after pass 2 — the target never materialized.
791+
this.recordDeferredError(deferred, allResults, allErrors,
792+
`Deferred reference unresolved after pass 2: ${deferred.objectName}.${deferred.field} = '${deferred.attemptedValue}' → ${deferred.targetObject}.${deferred.targetField} not found`);
797793
}
798794
}
799795
}
800796

797+
/**
798+
* Record a pass-2 (deferred) reference failure as a first-class error: it
799+
* lands in the object's per-result `errors`, bumps its `errored` count (so
800+
* `summary.totalErrored` is truthful), and joins `allErrors` (so the load
801+
* reports `success: false`). Both pass-2 failure modes — target still
802+
* missing, or the back-fill write threw — route through here so neither can
803+
* leave an incomplete relationship reported as a clean load (framework#2805).
804+
*/
805+
private recordDeferredError(
806+
deferred: DeferredUpdate,
807+
allResults: SeedLoadResult[],
808+
allErrors: ReferenceResolutionError[],
809+
message: string,
810+
): void {
811+
const error: ReferenceResolutionError = {
812+
sourceObject: deferred.objectName,
813+
field: deferred.field,
814+
targetObject: deferred.targetObject,
815+
targetField: deferred.targetField,
816+
attemptedValue: deferred.attemptedValue,
817+
recordIndex: deferred.recordIndex,
818+
message,
819+
};
820+
const resultEntry = allResults.find(r => r.object === deferred.objectName);
821+
if (resultEntry) {
822+
resultEntry.errors.push(error);
823+
resultEntry.errored++;
824+
}
825+
allErrors.push(error);
826+
}
827+
801828
// ==========================================================================
802829
// Internal: Write Operations
803830
// ==========================================================================

0 commit comments

Comments
 (0)