Skip to content

Commit 5522a2a

Browse files
committed
fix(core): validate writeBatch result contract in bulkWrite (#3151)
bulkWrite correlated writeBatch's return to input rows purely by position, with no check that it returned one record per row. A driver that returned fewer rows silently backfilled `record: undefined` and reported phantom successes; a longer return dropped records; a non-array return marked the whole batch ok with no records. Guard the return right after the batch write succeeds: a short / long / non-array result now throws ERR_BULK_RESULT_MISMATCH, which falls into the existing per-row degradation path (each row re-attempted via writeOne) instead of being trusted. The error is thrown outside withRetry and its message avoids any transient signature, so the batch is never retried on it. Also document bulkWrite's at-least-once delivery semantics in the module header, ahead of the idempotency work in #3149. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
1 parent 3a18b60 commit 5522a2a

2 files changed

Lines changed: 74 additions & 0 deletions

File tree

packages/core/src/utils/bulk-write.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,50 @@ describe('bulkWrite', () => {
112112
expect(writeOne).not.toHaveBeenCalled();
113113
expect(results[0]).toMatchObject({ index: 0, ok: false });
114114
});
115+
116+
it('rejects a short writeBatch return as a failed batch and degrades to per-row (#3151)', async () => {
117+
// Driver dropped a row from its RETURNING set: 2-row batch, 1 record back.
118+
const writeBatch = vi.fn(async (batch: { n: number }[]) => batch.slice(1).map((r) => ({ id: `r${r.n}` })));
119+
const writeOne = vi.fn(async (row: { n: number }) => ({ id: `r${row.n}` }));
120+
121+
const results = await bulkWrite([{ n: 1 }, { n: 2 }], { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
122+
123+
expect(writeBatch).toHaveBeenCalledTimes(1); // mismatch is NOT transient — no batch retry
124+
expect(writeOne).toHaveBeenCalledTimes(2); // degraded to per-row instead of phantom success
125+
expect(results.every((r) => r.ok)).toBe(true);
126+
expect(results.map((r) => r.record)).toEqual([{ id: 'r1' }, { id: 'r2' }]);
127+
});
128+
129+
it('rejects a non-array writeBatch return and degrades to per-row (#3151)', async () => {
130+
const writeBatch = vi.fn(async () => undefined as unknown as { id: string }[]);
131+
const writeOne = vi.fn(async (row: { n: number }) => ({ id: `r${row.n}` }));
132+
133+
const results = await bulkWrite([{ n: 1 }, { n: 2 }], { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
134+
135+
expect(writeOne).toHaveBeenCalledTimes(2);
136+
expect(results.every((r) => r.ok)).toBe(true);
137+
});
138+
139+
it('rejects an over-long writeBatch return and degrades to per-row (#3151)', async () => {
140+
const writeBatch = vi.fn(async (batch: { n: number }[]) => [...batch, { n: 999 }].map((r) => ({ id: `r${r.n}` })));
141+
const writeOne = vi.fn(async (row: { n: number }) => ({ id: `r${row.n}` }));
142+
143+
const results = await bulkWrite([{ n: 1 }, { n: 2 }], { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
144+
145+
expect(writeOne).toHaveBeenCalledTimes(2);
146+
expect(results.every((r) => r.ok)).toBe(true);
147+
});
148+
149+
it('surfaces ERR_BULK_RESULT_MISMATCH on a single-row batch with the wrong return count (#3151)', async () => {
150+
const writeBatch = vi.fn(async () => [] as { id: string }[]); // empty for a 1-row batch
151+
const writeOne = vi.fn(async () => ({ id: 'unused' }));
152+
153+
const results = await bulkWrite([{ n: 1 }], { batchSize: 10, writeBatch, writeOne, sleep: noopSleep });
154+
155+
expect(writeOne).not.toHaveBeenCalled(); // single-row batch failure IS the row's final result
156+
expect(results[0].ok).toBe(false);
157+
expect((results[0].error as { code?: string })?.code).toBe('ERR_BULK_RESULT_MISMATCH');
158+
});
115159
});
116160

117161
describe('withTransientRetry', () => {

packages/core/src/utils/bulk-write.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@
2626
* can reassemble output in input order even though rows are processed in
2727
* batches (and a batch's flush may be interleaved with other, immediate,
2828
* per-row work such as updates).
29+
*
30+
* Delivery semantics: **at-least-once**. Transient retry and per-row
31+
* degradation both RE-RUN a write whose outcome was unknown — e.g. a turso
32+
* `fetch failed` that arrived *after* the row was already committed
33+
* (framework#3149), or a result-count mismatch that voids the batch
34+
* (framework#3151). A caller that needs exactly-once must make its
35+
* `writeBatch`/`writeOne` idempotent; both receive an `attempt` counter for
36+
* exactly this — see the natural-key recheck the seed loader and import
37+
* runner perform on `attempt > 1`. `writeBatch` MUST also resolve exactly one
38+
* record per input row, in input order: a short / long / non-array return is
39+
* rejected as a failed batch (framework#3151), never silently backfilled.
2940
*/
3041

3142
export interface BulkWriteRowResult<TRecord = any> {
@@ -162,6 +173,25 @@ export async function bulkWrite<TRow, TRecord = any>(
162173
const batch = rows.slice(start, start + batchSize);
163174
try {
164175
const records = await withRetry(() => opts.writeBatch(batch), retryOpts);
176+
// Contract guard (framework#3151): `writeBatch` must resolve one record
177+
// per input row. A short / long / non-array return breaks the positional
178+
// correlation below, so backfilling it would report phantom successes
179+
// (`record: undefined`) or drop records. Treat the whole batch as failed
180+
// and fall through to per-row degradation (each row re-attempted via
181+
// `writeOne`, which under an idempotent caller rechecks before writing).
182+
// The message deliberately avoids any transient signature so this never
183+
// reads as a retryable blip — and it is thrown *outside* `withRetry`, so
184+
// the batch is not retried on it.
185+
if (!Array.isArray(records) || records.length !== batch.length) {
186+
throw Object.assign(
187+
new Error(
188+
`bulkWrite: writeBatch returned ${
189+
Array.isArray(records) ? `${records.length} record(s)` : String(typeof records)
190+
} for a ${batch.length}-row batch — treating batch as failed`,
191+
),
192+
{ code: 'ERR_BULK_RESULT_MISMATCH' },
193+
);
194+
}
165195
for (let i = 0; i < batch.length; i++) {
166196
results[start + i] = { index: start + i, ok: true, record: records[i] };
167197
}

0 commit comments

Comments
 (0)