From feed44d54255f5ead1538e9080be20aefcf4f7c2 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 09:21:36 -0600 Subject: [PATCH 01/14] Distinguish replication confirmation failure from a local write failure Wait for linked local commits before reporting replication confirmation errors. Preserve their HTTP status and code, expose replicationUnconfirmed in REST and operations responses, and prevent retryable hints from replaying committed writes. Co-Authored-By: OpenAI Codex GPT-6.1 --- DESIGN.md | 1 + resources/DESIGN.md | 17 +- resources/DatabaseTransaction.ts | 42 ++- resources/LMDBTransaction.ts | 10 +- server/REST.ts | 1 + server/serverHelpers/serverHandlers.js | 14 +- .../replicationConfirmationOutcome.test.js | 325 ++++++++++++++++++ .../serverHelpers/serverErrorHandler.test.js | 84 ++++- utility/errors/hdbError.ts | 22 ++ 9 files changed, 506 insertions(+), 10 deletions(-) create mode 100644 unitTests/resources/replicationConfirmationOutcome.test.js diff --git a/DESIGN.md b/DESIGN.md index d4cc31bd9a..e071050af3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -72,6 +72,7 @@ Index of the design notes for the harper core: one line per note, grouped by the - [A local-only write marks both the record and its audit entry, and replay preserves it (`Table.ts` internal writes, `replayLogs.ts`)](resources/DESIGN.md#a-local-only-write-marks-both-the-record-and-its-audit-entry-and-replay-preserves-it-tablets-internal-writes-replaylogsts) — Mutations set `LOCAL_ONLY` on both persisted forms; messages set only their audit entry and preserve the row bit; crash replay reads the bit back from the entry (harper#2711). - [A record's residency is the listed nodes plus the writer, and a patch over a stub stays a stub (`Table.ts` `_writeUpdate`, harper#2257)](resources/DESIGN.md#a-records-residency-is-the-listed-nodes-plus-the-writer-and-a-patch-over-a-stub-stays-a-stub-tablets-_writeupdate-harper2257) — A local write never sheds the writer's complete copy; a patch onto an `INVALIDATED` stub is stored as a stub, never a complete row. - [`replicatedConfirmation` counts only the residents of each written record (`DatabaseTransaction` `confirmationTarget`, harper-pro#1031)](resources/DESIGN.md#replicatedconfirmation-counts-only-the-residents-of-each-written-record-databasetransaction-confirmationtarget-harper-pro1031) — The confirmation callback gets one other-resident list per distinct residency; a count any one list cannot meet fails before the commit. +- [Replication confirmation failure means the linked writes committed locally](resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally) — Confirmation rejects only after local completions; its error carries `replicationUnconfirmed: true` in code and HTTP responses. - [This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`)](resources/DESIGN.md#this-nodes-short-id-is-0-confirmed-per-audit-store-nodeidmappingts-getthisnodeid) — Audited writes skip re-reading the id-mapping record while the node name is unchanged; the per-worker confirmation expires so another worker's remap is picked up. ## resources/audit-retention.md — audit retention and cleanup diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 1fbb921c22..157d9178fa 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -985,10 +985,23 @@ submission the commit rechecks the lists (confirmation may have been requested a and runs the feasibility check the replication layer registered with `replicationConfirmation` (harper-pro: the recipient bound). A table source's `before` hook (`source.put`/`patch`) has already run by then, because record-based placement needs the merged record; the guarantee covers Harper's stored data, not the source. -An error after the native commit becomes a rejected completion, so the commit's bookkeeping still runs. -LMDB is untouched: its commit never reaches a registered confirmation callback. Enforced by +An error after the native commit is observed after local completions, so bookkeeping and chained commits +finish before a confirmation failure surfaces. LMDB's production confirmation registry remains unregistered. Enforced by `unitTests/resources/residencyConfirmation.test.js`. +## Replication confirmation failure means the linked writes committed locally + +`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook +failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` +waits for local work first, deferring even a child's confirmation failure. Local write/flush failures +remain prompt and unmarked. The wrapper retains the original public code, message and status through +`cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat +committed writes. REST problem details and operations error bodies expose the flag. This is local commit, +the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot +resume writes. Confirmation registration and count propagation are unchanged (LMDB and later databases +are still not automatically confirmed). Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` +and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. + ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) Every audited write stamps the writing node's short id, so `getThisNodeId` sits on the write path. `getIdMappingRecord` leaves the current node name (`getThisNodeName()`, which `server.hostname` returns) at id `0` whenever it runs, remapping a previous local name to a fresh id on a rename, and no other mapping writer assigns `0`. `getThisNodeId` therefore reads and unpacks the mapping record only to confirm that, and caches the confirmation per audit store and node name; a different name (after `clearThisNodeName()`) takes the read path again, which performs the remap. The confirmation is per worker while the record is shared, and a worker that resolved a different name remaps the record under the others, so it expires after `THIS_NODE_ID_CONFIRM_MS` (monotonic time) and `invalidateNodeNames` drops it wherever the record is written. Pinned by `unitTests/resources/thisNodeId.test.js`. diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 774b83abe2..4d4e3a6209 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -5,6 +5,7 @@ import { ClientError, DatabaseClosingError, DatabaseDrainTimeoutError, + ReplicationConfirmationError, ServerError, TransactionCommitConflictTimeoutError, } from '../utility/errors/hdbError.ts'; @@ -487,6 +488,38 @@ export function replicationConfirmation(callback, feasibility?: typeof confirmat confirmReplication = callback; confirmationFeasibility = feasibility; } + +export function confirmReplicationAfterCommit(callback: () => unknown): Promise { + let result; + try { + result = callback(); + } catch (error) { + result = Promise.reject(error); + } + const confirmation = Promise.resolve(result).then( + () => {}, + (error) => { + throw new ReplicationConfirmationError(error); + } + ); + confirmation.catch(() => {}); + return confirmation; +} + +export function settleCommitCompletions(completions: unknown[], confirmation?: Promise): Promise { + let deferredConfirmationError: ReplicationConfirmationError; + const localCompletions = completions.map((completion) => + Promise.resolve(completion).catch((error) => { + if (error instanceof ReplicationConfirmationError) deferredConfirmationError ??= error; + else throw error; + }) + ); + // A confirmation failure must not abort a chained local commit that has yet to land. + return Promise.all(localCompletions).then(() => { + if (deferredConfirmationError) throw deferredConfirmationError; + return confirmation; + }); +} let txnExpiration = envMngr.get(CONFIG_PARAMS.STORAGE_MAXTRANSACTIONOPENTIME) ?? 30000; class StartedTransaction extends Error {} @@ -2964,6 +2997,7 @@ export class DatabaseTransaction implements Transaction { const store = this.writes[0]?.store; if (store) completions.push(store.flushed); } + let confirmation: Promise; if (this.replicatedConfirmation) { // if we want to wait for replication confirmation, we need to track the transaction times // and when replication notifications come in, we count the number of confirms until we reach the desired number @@ -2975,7 +3009,7 @@ export class DatabaseTransaction implements Transaction { ? this.confirmationTarget(committedLogKey, committedTransaction) : undefined; if (target) { - completions.push( + confirmation = confirmReplicationAfterCommit(() => confirmReplication( databaseName, target.version, @@ -3021,7 +3055,11 @@ export class DatabaseTransaction implements Transaction { !this.hasActiveCommitHandle() ) this.timestamp = 0; // reset the timestamp as well - return Promise.all(completions).then( + const completed = + confirmation || this.next + ? settleCommitCompletions(completions, confirmation) + : Promise.all(completions); + return completed.then( () => { // Only once the chained store's commit has settled, as on the synchronous path: a // partially failed mid-scope commit must not leave the scope resumable. diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index ef800cc98d..5351446ea3 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -7,6 +7,8 @@ import { shouldSpareCommitPhase, requestAbortedError, transactionOpenTooLongError, + confirmReplicationAfterCommit, + settleCommitCompletions, type CommitOptions, type TransactionWrite, type CommitResolution, @@ -329,6 +331,7 @@ export class LMDBTransaction extends DatabaseTransaction { if (resolution) { return resolution.then((resolution) => { if (resolution) { + const hasNext = Boolean(this.next); if (this.next) { completions.push(this.next.commit({ ...options, continuation: true })); } @@ -336,13 +339,14 @@ export class LMDBTransaction extends DatabaseTransaction { const store = this.writes[0]?.store; if (store) completions.push(store.flushed); } + let confirmation: Promise; if (this.replicatedConfirmation) { // if we want to wait for replication confirmation, we need to track the transaction times // and when replication notifications come in, we count the number of confirms until we reach the desired number const databaseName = this.writes[0]?.store.rootStore.databaseName; const lastEntry = this.lastConfirmableEntry(); if (confirmReplication && databaseName && lastEntry) - completions.push( + confirmation = confirmReplicationAfterCommit(() => confirmReplication(databaseName, (lastEntry as any).localTime, this.replicatedConfirmation) ); } @@ -359,7 +363,9 @@ export class LMDBTransaction extends DatabaseTransaction { this.clearWrites(); this.timestamp = 0; this.next = null; - return Promise.all(completions).then(() => { + const completed = + confirmation || hasNext ? settleCommitCompletions(completions, confirmation) : Promise.all(completions); + return completed.then(() => { return { txnTime, }; diff --git a/server/REST.ts b/server/REST.ts index a79395420c..109eb00507 100644 --- a/server/REST.ts +++ b/server/REST.ts @@ -480,6 +480,7 @@ async function http(request: Request, nextHandler, resources: Resources, httpOpt status, detail: error.detail, instance: request.url, + ...(error.replicationUnconfirmed === true ? { replicationUnconfirmed: true } : {}), }; const responseObject = { diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index ddc602e93b..038e1376bc 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -81,6 +81,7 @@ function serverErrorHandler(error, req, resp) { if (error.statusCode) { if (typeof error.http_resp_msg !== 'object') { const body = { error: error.http_resp_msg || error.message }; + if (error.replicationUnconfirmed === true) body.replicationUnconfirmed = true; // surface the machine-readable signal for errors that declare retryability (e.g. a // still-building index, IndexRebuildingError) so API callers can distinguish and retry if (error.retryable !== undefined) { @@ -89,13 +90,22 @@ function serverErrorHandler(error, req, resp) { } return resp.code(error.statusCode).send(body); } - return resp.code(error.statusCode).send(error.http_resp_msg); + return resp + .code(error.statusCode) + .send( + error.replicationUnconfirmed === true + ? { ...error.http_resp_msg, replicationUnconfirmed: true, retryable: false } + : error.http_resp_msg + ); } const statusCode = error.statusCode ? error.statusCode : hdbErrors.HTTP_STATUS_CODES.INTERNAL_SERVER_ERROR; if (typeof error === 'string') { return resp.code(statusCode).send({ error: error }); } - return resp.code(statusCode).send(error.message ? { error: error.message } : error); + const body = error.message ? { error: error.message } : error; + return resp + .code(statusCode) + .send(error.replicationUnconfirmed === true ? { ...body, replicationUnconfirmed: true } : body); } function reqBodyValidationHandler(req, resp, done) { diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js new file mode 100644 index 0000000000..260d7bac0f --- /dev/null +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -0,0 +1,325 @@ +const assert = require('node:assert'); +const { setupTestDBPath } = require('../testUtils'); +const { waitFor } = require('../waitFor'); +const { table } = require('#src/resources/databases'); +const { setMainIsWorker } = require('#js/server/threads/manageThreads'); +const { transaction, contextStorage } = require('#src/resources/transaction'); +const { + replicationConfirmation: rocksConfirmation, + settleCommitCompletions, +} = require('#src/resources/DatabaseTransaction'); +const { replicationConfirmation: lmdbConfirmation } = require('#src/resources/LMDBTransaction'); +const { ReplicationConfirmationError, ServerError } = require('#src/utility/errors/hdbError'); +const { Resource } = require('#src/resources/Resource'); +const { Resources } = require('#src/resources/Resources'); +const { handleApplication } = require('#src/server/REST'); + +const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; +const registerConfirmation = isLMDB ? lmdbConfirmation : rocksConfirmation; + +describe('Replication confirmation failure after a local commit', () => { + let First, Second; + let sequence = 0; + const nextId = () => `confirmation-${sequence++}`; + + before(() => { + setupTestDBPath(); + setMainIsWorker(true); + First = table({ + database: 'confirmation-outcome-first', + table: 'Records', + attributes: [{ name: 'id', isPrimaryKey: true }], + }); + Second = table({ + database: 'confirmation-outcome-second', + table: 'Records', + attributes: [{ name: 'id', isPrimaryKey: true }], + }); + assert.notEqual(First.primaryStore.path, Second.primaryStore.path); + }); + afterEach(() => registerConfirmation(undefined)); + + function requestConfirmation(context) { + let link = context.transaction; + while (link.db?.path !== First.primaryStore.path) link = link.next; + // LMDB's production registry is unregistered; these tests explicitly exercise its commit path. + link.replicatedConfirmation = 1; + } + + async function confirmedWrite(id) { + const context = { replicatedConfirmation: 1 }; + return transaction(context, async () => { + await First.put({ id }); + requestConfirmation(context); + }); + } + + for (const exclusive of isLMDB ? [false, true] : [false]) { + for (const synchronous of [false, true]) { + it(`marks ${synchronous ? 'synchronous' : 'asynchronous'} confirmation failure${exclusive ? ' (exclusive LMDB)' : ''}`, async () => { + const id = nextId(); + const failure = new ServerError('Replication confirmation timed out', 504); + failure.code = 'PEER_TIMEOUT'; + failure.retryable = true; + Object.freeze(failure); + registerConfirmation(() => { + if (synchronous) throw failure; + return Promise.reject(failure); + }); + const previousRisk = First.primaryStore.retryRisk; + if (exclusive) First.primaryStore.retryRisk = 101; + try { + await assert.rejects(confirmedWrite(id), (error) => { + assert.ok(error instanceof ReplicationConfirmationError); + assert.strictEqual(error.cause, failure); + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual(error.retryable, false); + assert.strictEqual(error.statusCode, 504); + assert.strictEqual(error.code, 'PEER_TIMEOUT'); + assert.strictEqual(error.message, failure.message); + return true; + }); + assert.strictEqual((await First.get(id)).id, id); + assert.strictEqual(failure.replicationUnconfirmed, undefined); + } finally { + First.primaryStore.retryRisk = previousRisk; + } + }); + } + } + + it('normalizes a primitive rejection', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject('peer unavailable')); + await assert.rejects(confirmedWrite(id), (error) => { + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual(error.cause, 'peer unavailable'); + assert.strictEqual(error.statusCode, 500); + return true; + }); + assert.strictEqual((await First.get(id)).id, id); + }); + + it('does not mark a shared error later thrown by a local operation', async () => { + const failure = new ServerError('Shared failure', 504); + registerConfirmation(() => Promise.reject(failure)); + await assert.rejects(confirmedWrite(nextId()), (error) => error.cause === failure); + assert.strictEqual(failure.replicationUnconfirmed, undefined); + await assert.rejects( + async () => + transaction({}, () => { + throw failure; + }), + (error) => error === failure && error.replicationUnconfirmed === undefined + ); + }); + + it('retains successful confirmation and the existing callback arguments', async () => { + const id = nextId(); + const calls = []; + registerConfirmation((databaseName, version, count) => calls.push({ databaseName, version, count })); + await confirmedWrite(id); + assert.strictEqual(calls.length, 1); + assert.strictEqual(calls[0].databaseName, First.primaryStore.rootStore.databaseName); + assert.strictEqual(calls[0].count, 1); + assert.ok(calls[0].version > 0); + assert.strictEqual((await First.get(id)).id, id); + }); + + it('leaves a pre-submission feasibility refusal unmarked and unwritten', async function () { + if (isLMDB) this.skip(); + const id = nextId(); + const failure = new ServerError('Confirmation count cannot be met', 400); + let waiterCalled = false; + rocksConfirmation( + () => (waiterCalled = true), + () => { + throw failure; + } + ); + await assert.rejects(confirmedWrite(id), (error) => error === failure); + assert.strictEqual(failure.replicationUnconfirmed, undefined); + assert.strictEqual(waiterCalled, false); + assert.equal(await First.get(id), null); + }); + + it('waits for a held second database before surfacing confirmation failure', async () => { + const id = nextId(); + const gate = Promise.withResolvers(); + let entered = false; + class Delayed extends Second {} + Delayed.sourcedFrom({ + get() {}, + put() { + entered = true; + return gate.promise; + }, + }); + let confirmationStarted = false; + registerConfirmation(() => { + confirmationStarted = true; + throw new ServerError('Peer timeout', 504); + }); + const context = { replicatedConfirmation: 1 }; + let settled = false; + const committing = transaction(context, async () => { + await First.put({ id }); + requestConfirmation(context); + await Delayed.put({ id }); + }).then( + () => { + settled = true; + }, + (error) => { + settled = true; + return error; + } + ); + try { + await waitFor(() => entered && confirmationStarted); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(settled, false, 'confirmation must not interrupt the held local commit'); + gate.resolve(); + await waitFor(() => settled); + const error = await committing; + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual((await First.get(id)).id, id); + assert.strictEqual((await Second.get(id)).id, id); + } finally { + gate.resolve(); + await waitFor(() => settled); + await committing; + } + }); + + for (const confirmation of [false, true]) { + it(`surfaces a later local failure promptly${confirmation ? ' even when confirmation never settles' : ' without confirmation'}`, async () => { + const id = nextId(); + const failure = new ServerError('Later source failed', 502); + class Failing extends Second {} + Failing.sourcedFrom({ get() {}, put: () => Promise.reject(failure) }); + let started = false; + registerConfirmation(() => { + started = true; + return new Promise(() => {}); + }); + const context = confirmation ? { replicatedConfirmation: 1 } : {}; + let outcome; + let settled = false; + const committing = transaction(context, async () => { + await First.put({ id }); + if (confirmation) requestConfirmation(context); + await Failing.put({ id }); + }).then( + () => (settled = true), + (error) => { + outcome = error; + settled = true; + } + ); + await waitFor(() => settled); + await committing; + assert.strictEqual(outcome, failure); + assert.strictEqual(outcome.replicationUnconfirmed, undefined); + assert.strictEqual(started, confirmation); + assert.strictEqual((await First.get(id)).id, id); + assert.equal(await Second.get(id), null); + }); + } + + it('defers a child confirmation failure until other local completions settle', async () => { + const gate = Promise.withResolvers(); + const failure = new ReplicationConfirmationError(new ServerError('Child confirmation failed', 504)); + let settled = false; + const completing = settleCommitCompletions([Promise.reject(failure), gate.promise]).catch((error) => { + settled = true; + return error; + }); + try { + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(settled, false); + gate.resolve(); + await waitFor(() => settled); + assert.strictEqual(await completing, failure); + } finally { + gate.resolve(); + } + }); + + it('reports a local failure ahead of a child confirmation failure', async () => { + const localFailure = new ServerError('Local flush failed', 500); + const childFailure = new ReplicationConfirmationError(new ServerError('Child confirmation failed', 504)); + await assert.rejects( + settleCommitCompletions([Promise.reject(childFailure), Promise.reject(localFailure)]), + (error) => error === localFailure + ); + }); + + it('surfaces a child confirmation failure even if the parent waiter never settles', async () => { + const id = nextId(); + const context = { replicatedConfirmation: 1 }; + registerConfirmation((databaseName) => + databaseName === First.primaryStore.rootStore.databaseName + ? new Promise(() => {}) + : Promise.reject(new ServerError('Child confirmation failed', 504)) + ); + let settled = false; + let outcome; + const committing = transaction(context, async () => { + await First.put({ id }); + requestConfirmation(context); + await Second.put({ id }); + let child = context.transaction; + while (child.db?.path !== Second.primaryStore.path) child = child.next; + child.replicatedConfirmation = 1; + }).then( + () => (settled = true), + (error) => { + settled = true; + outcome = error; + } + ); + await waitFor(() => settled); + await committing; + assert.strictEqual(outcome.replicationUnconfirmed, true); + assert.strictEqual(outcome.message, 'Child confirmation failed'); + assert.strictEqual((await First.get(id)).id, id); + assert.strictEqual((await Second.get(id)).id, id); + }); + + it('publishes the flag in REST problem details while retaining the original public code', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); + class Signup extends Resource { + static loadAsInstance = false; + async post(target) { + await First.put({ id: target.id }); + requestConfirmation(contextStorage.getStore()); + return 'created'; + } + } + const resources = new Resources(); + resources.set('Signup', Signup); + let listener; + handleApplication({ + resources, + options: { getAll: () => ({ webSocket: false }) }, + routeFor: () => ({ urlPath: '/confirmation-outcome' }), + server: { http: (handler) => (listener = handler) }, + }); + const response = await listener({ + method: 'POST', + url: `/Signup/${id}`, + pathname: `/Signup/${id}`, + ip: '203.0.113.7', + user: { username: 'admin', role: { permission: { super_user: true } } }, + headers: { asObject: {}, get: () => undefined }, + }); + assert.strictEqual(response.status, 504); + const body = JSON.parse(response.body); + assert.strictEqual(body.replicationUnconfirmed, true); + assert.strictEqual(body.code, 'ServerError'); + assert.strictEqual(body.title, 'Peer timeout'); + assert.strictEqual((await First.get(id)).id, id); + }); +}); diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index e325777480..aa4a23a7c4 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -2,8 +2,12 @@ const assert = require('assert'); const fastify = require('fastify'); -const { hdbErrors } = require('#src/utility/errors/hdbError'); -const { serverErrorHandler } = require('#js/server/serverHelpers/serverHandlers'); +const { hdbErrors, ReplicationConfirmationError, ServerError, HdbError } = require('#src/utility/errors/hdbError'); +const { serverErrorHandler, handlePostRequest } = require('#js/server/serverHelpers/serverHandlers'); +const { setupTestDBPath } = require('../../testUtils'); +const { table } = require('#src/resources/databases'); +const { setMainIsWorker } = require('#js/server/threads/manageThreads'); +const { replicationConfirmation } = require('#src/resources/DatabaseTransaction'); describe('serverErrorHandler', function () { it('answers an ordinary 500 when a handler rejects with no error', async function () { @@ -20,4 +24,80 @@ describe('serverErrorHandler', function () { await app.close(); } }); + + for (const structured of [false, true]) { + it(`publishes a confirmation failure${structured ? ' with structured HdbError details' : ''}`, async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + const cause = structured + ? new HdbError(new Error('Peer timeout'), { error: 'Peer timeout', peers: ['node-a'], retryable: true }, 504) + : new ServerError('Peer timeout', 504); + cause.retryable = true; + const failure = new ReplicationConfirmationError(cause); + app.post('/', async () => { + throw failure; + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.strictEqual(response.statusCode, 504); + assert.strictEqual(response.json().replicationUnconfirmed, true); + assert.notEqual(response.json().retryable, true); + assert.strictEqual(response.json().error, 'Peer timeout'); + if (structured) assert.deepStrictEqual(response.json().peers, ['node-a']); + assert.strictEqual(cause.replicationUnconfirmed, undefined); + } finally { + await app.close(); + } + }); + } + + it('omits the flag from an ordinary local error', async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + app.post('/', async () => { + throw new ServerError('Local failure', 500); + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.deepStrictEqual(response.json(), { error: 'Local failure' }); + } finally { + await app.close(); + } + }); + + it('retains the flag through the real operations insert dispatch and a local commit', async () => { + setupTestDBPath(); + setMainIsWorker(true); + const Records = table({ + database: 'confirmation-outcome-operations', + table: 'Records', + attributes: [{ name: 'id', isPrimaryKey: true }], + }); + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + app.addHook('preHandler', async (request) => { + request.body.hdb_user = { username: 'admin', role: { permission: { super_user: true } } }; + }); + app.post('/', handlePostRequest); + replicationConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); + try { + const response = await app.inject({ + method: 'POST', + url: '/', + payload: { + operation: 'insert', + schema: 'confirmation-outcome-operations', + table: 'Records', + records: [{ id: 'committed' }], + replicatedConfirmation: 1, + }, + }); + assert.strictEqual(response.statusCode, 504, response.body); + assert.strictEqual(response.json().replicationUnconfirmed, true); + assert.strictEqual((await Records.get('committed')).id, 'committed'); + } finally { + replicationConfirmation(undefined); + await app.close(); + } + }); }); diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index d75f45ad9d..56c96262cb 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -93,6 +93,28 @@ export class ServerError extends Error { } } +export class ReplicationConfirmationError extends ServerError { + replicationUnconfirmed = true; + retryable = false; + code: string; + http_resp_msg: unknown; + detail: unknown; + + constructor(cause: any) { + super( + typeof cause?.message === 'string' + ? cause.message + : 'Replication confirmation failed after the local write committed', + cause?.statusCode + ); + this.name = 'ReplicationConfirmationError'; + this.cause = cause; + this.code = cause?.code ?? (cause instanceof Error ? cause.constructor.name : this.name); + this.http_resp_msg = cause?.http_resp_msg; + this.detail = cause?.detail; + } +} + /** Retryable admission failure when a derived index cannot meet a write or query lag budget. */ export class DerivedIndexLagError extends ServerError { code: string; From 8003ba0c4cff1eb190da8744890ef7e005cd9492 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 09:49:47 -0600 Subject: [PATCH 02/14] Preserve confirmation error codes across JavaScript realms Copy the original constructor name without relying on instanceof Error, and accept its existing status alias. Verify with a real foreign-realm error through the registered confirmation hook. Co-Authored-By: OpenAI Codex GPT-6.1 --- .../replicationConfirmationOutcome.test.js | 22 +++++++++++++++++++ utility/errors/hdbError.ts | 5 +++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 260d7bac0f..75380a42ee 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -1,4 +1,5 @@ const assert = require('node:assert'); +const { runInNewContext } = require('node:vm'); const { setupTestDBPath } = require('../testUtils'); const { waitFor } = require('../waitFor'); const { table } = require('#src/resources/databases'); @@ -100,6 +101,27 @@ describe('Replication confirmation failure after a local commit', () => { assert.strictEqual((await First.get(id)).id, id); }); + it('preserves the public code and status of an error from another JavaScript realm', async () => { + const failure = runInNewContext(` + class ForeignConfirmationError extends Error { + constructor() { + super('Foreign peer failure'); + this.status = 504; + } + } + new ForeignConfirmationError(); + `); + assert.strictEqual(failure instanceof Error, false); + registerConfirmation(() => Promise.reject(failure)); + await assert.rejects(confirmedWrite(nextId()), (error) => { + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual(error.statusCode, 504); + assert.strictEqual(error.code, 'ForeignConfirmationError'); + assert.strictEqual(error.cause, failure); + return true; + }); + }); + it('does not mark a shared error later thrown by a local operation', async () => { const failure = new ServerError('Shared failure', 504); registerConfirmation(() => Promise.reject(failure)); diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index 56c96262cb..28def899b1 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -105,11 +105,12 @@ export class ReplicationConfirmationError extends ServerError { typeof cause?.message === 'string' ? cause.message : 'Replication confirmation failed after the local write committed', - cause?.statusCode + cause?.statusCode ?? cause?.status ); this.name = 'ReplicationConfirmationError'; this.cause = cause; - this.code = cause?.code ?? (cause instanceof Error ? cause.constructor.name : this.name); + this.code = + cause?.code ?? (cause !== null && typeof cause === 'object' ? cause.constructor?.name : undefined) ?? this.name; this.http_resp_msg = cause?.http_resp_msg; this.detail = cause?.detail; } From d9937a67d82dea0673030090740d1d65958c4fbb Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 11:15:00 -0600 Subject: [PATCH 03/14] Keep confirmation outcomes scoped and visible in streamed errors Preserve prompt local failures and the ordinary unconfirmed commit path. Strip foreign nested confirmation markers before rolling back pending writes; expose the nonretryable outcome in structured and streaming responses. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 15 ++- resources/DatabaseTransaction.ts | 4 +- resources/LMDBTransaction.ts | 8 +- resources/transaction.ts | 7 ++ server/REST.ts | 2 +- server/serverHelpers/JSONStream.ts | 10 +- server/serverHelpers/contentTypes.ts | 10 +- server/serverHelpers/serverHandlers.js | 22 ++--- .../replicationConfirmationOutcome.test.js | 92 +++++++++++++++++++ .../serverHelpers/serverErrorHandler.test.js | 29 +++++- utility/errors/hdbError.ts | 15 +++ 11 files changed, 191 insertions(+), 23 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 157d9178fa..e4b8e29e9c 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -985,8 +985,9 @@ submission the commit rechecks the lists (confirmation may have been requested a and runs the feasibility check the replication layer registered with `replicationConfirmation` (harper-pro: the recipient bound). A table source's `before` hook (`source.put`/`patch`) has already run by then, because record-based placement needs the merged record; the guarantee covers Harper's stored data, not the source. -An error after the native commit is observed after local completions, so bookkeeping and chained commits -finish before a confirmation failure surfaces. LMDB's production confirmation registry remains unregistered. Enforced by +A confirmation-hook error after the native commit is observed after local completions, so bookkeeping and chained commits +finish before a confirmation failure surfaces. LMDB's production registry is unregistered and its write-bearing +child does not inherit the root's confirmation count. Enforced by `unitTests/resources/residencyConfirmation.test.js`. ## Replication confirmation failure means the linked writes committed locally @@ -996,12 +997,20 @@ failure. `confirmReplicationAfterCommit` observes the hook immediately, and `set waits for local work first, deferring even a child's confirmation failure. Local write/flush failures remain prompt and unmarked. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat -committed writes. REST problem details and operations error bodies expose the flag. This is local commit, +committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Confirmation registration and count propagation are unchanged (LMDB and later databases are still not automatically confirmed). Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. +A confirmation error from an independent nested transaction cannot attest to an enclosing scope's +pending writes: the commit and callback failure funnels remove its flag before aborting those writes, +keeping the original error in `cause` and refusing automatic retry because the nested write did land. + +Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect +`cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) +render the new class name. Object responses preserve the original public code alongside the flag. + ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) Every audited write stamps the writing node's short id, so `getThisNodeId` sits on the write path. `getIdMappingRecord` leaves the current node name (`getThisNodeName()`, which `server.hostname` returns) at id `0` whenever it runs, remapping a previous local name to a fresh id on a rename, and no other mapping writer assigns `0`. `getThisNodeId` therefore reads and unpacks the mapping record only to confirm that, and caches the confirmation per audit store and node name; a different name (after `clearThisNodeName()`) takes the read path again, which performs the remap. The confirmation is per worker while the record is shared, and a worker that resolved a different name remaps the record under the others, so it expires after `THIS_NODE_ID_CONFIRM_MS` (monotonic time) and `invalidateNodeNames` drops it wherever the record is written. Pinned by `unitTests/resources/thisNodeId.test.js`. diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 4d4e3a6209..a073d8e0fa 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -6,6 +6,7 @@ import { DatabaseClosingError, DatabaseDrainTimeoutError, ReplicationConfirmationError, + withoutReplicationUnconfirmed, ServerError, TransactionCommitConflictTimeoutError, } from '../utility/errors/hdbError.ts'; @@ -3056,7 +3057,7 @@ export class DatabaseTransaction implements Transaction { ) this.timestamp = 0; // reset the timestamp as well const completed = - confirmation || this.next + confirmation || (this.root ?? this).replicatedConfirmation ? settleCommitCompletions(completions, confirmation) : Promise.all(completions); return completed.then( @@ -3377,6 +3378,7 @@ export class DatabaseTransaction implements Transaction { } protected abortAfterCommitError(error, targetTransaction?: RocksTransaction): never { + if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) error = withoutReplicationUnconfirmed(error); try { const retryTxn = targetTransaction as RocksTransactionWithRetry | undefined; const retryTxnCommitted = this.isHandleCommitted(retryTxn); diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index 5351446ea3..025d3cfe6a 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -14,7 +14,7 @@ import { type CommitResolution, } from './DatabaseTransaction'; import { cleanupUnusedBlobs, collectRetainedFileIds } from './blob.ts'; -import { ServerError } from '../utility/errors/hdbError.ts'; +import { ServerError, withoutReplicationUnconfirmed } from '../utility/errors/hdbError.ts'; import { getNextMonotonicTime } from '../utility/lmdb/commonUtility.ts'; import * as harperLogger from '../utility/logging/harper_logger.ts'; import type { Context } from './ResourceInterface.ts'; @@ -331,7 +331,6 @@ export class LMDBTransaction extends DatabaseTransaction { if (resolution) { return resolution.then((resolution) => { if (resolution) { - const hasNext = Boolean(this.next); if (this.next) { completions.push(this.next.commit({ ...options, continuation: true })); } @@ -364,7 +363,9 @@ export class LMDBTransaction extends DatabaseTransaction { this.timestamp = 0; this.next = null; const completed = - confirmation || hasNext ? settleCommitCompletions(completions, confirmation) : Promise.all(completions); + confirmation || (this.root ?? this).replicatedConfirmation + ? settleCommitCompletions(completions, confirmation) + : Promise.all(completions); return completed.then(() => { return { txnTime, @@ -400,6 +401,7 @@ export class LMDBTransaction extends DatabaseTransaction { } private abortCommitFailure(error, next: DatabaseTransaction | undefined): never { + if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) error = withoutReplicationUnconfirmed(error); const nextWasAttached = Boolean(next && this.next === next); try { this.abort(true); diff --git a/resources/transaction.ts b/resources/transaction.ts index ea53cebcea..9542093503 100644 --- a/resources/transaction.ts +++ b/resources/transaction.ts @@ -8,6 +8,7 @@ import { } from './DatabaseTransaction.ts'; import { AsyncLocalStorage } from 'async_hooks'; import * as harperLogger from '../utility/logging/harper_logger.ts'; +import { withoutReplicationUnconfirmed } from '../utility/errors/hdbError.ts'; export const contextStorage = new AsyncLocalStorage(); @@ -104,6 +105,12 @@ export function transaction( abortAndThrow(error, true); } function abortAndThrow(error, callbackThrew: boolean): never { + if ( + callbackThrew && + error?.replicationUnconfirmed === true && + (transaction.hasPendingWrites() || transaction.isChainCommitting()) + ) + error = withoutReplicationUnconfirmed(error); // A commit attempt that has not reached its native outcome owns its own teardown — a handler that // fired txn.commit() without awaiting it can get here while it is still running, and aborting // would clear the writes it is committing and abort the handle it is committing them through. diff --git a/server/REST.ts b/server/REST.ts index 109eb00507..bf15eeaf22 100644 --- a/server/REST.ts +++ b/server/REST.ts @@ -480,7 +480,7 @@ async function http(request: Request, nextHandler, resources: Resources, httpOpt status, detail: error.detail, instance: request.url, - ...(error.replicationUnconfirmed === true ? { replicationUnconfirmed: true } : {}), + ...(error.replicationUnconfirmed === true ? { replicationUnconfirmed: true, retryable: false } : {}), }; const responseObject = { diff --git a/server/serverHelpers/JSONStream.ts b/server/serverHelpers/JSONStream.ts index 58b0eb7db9..e18ef8960f 100644 --- a/server/serverHelpers/JSONStream.ts +++ b/server/serverHelpers/JSONStream.ts @@ -9,6 +9,12 @@ const BIGINT_SERIALIZATION = { message: 'Cannot serialize BigInt to JSON' }; throw BIGINT_SERIALIZATION; }; const { errorToString } = harperLogger; +function streamError(error) { + return { + error: errorToString(error), + ...(error?.replicationUnconfirmed === true ? { replicationUnconfirmed: true, retryable: false } : {}), + }; +} export function streamAsJSON(value) { return new JSONStream({ value }); } @@ -58,7 +64,7 @@ class JSONStream extends Readable { (error) => { // try to properly serialize the error, but then finish the iterator harperLogger.warn('Error serializing in stream', error); - iteratorResult = { done: false, value: { error: errorToString(error) } }; + iteratorResult = { done: false, value: streamError(error) }; iterator = { next: () => ({ done: true }), }; @@ -67,7 +73,7 @@ class JSONStream extends Readable { ); } } catch (error) { - iteratorResult = { done: false, value: { error: errorToString(error) } }; + iteratorResult = { done: false, value: streamError(error) }; iterator = { next: () => ({ done: true }), }; diff --git a/server/serverHelpers/contentTypes.ts b/server/serverHelpers/contentTypes.ts index 93c8f96477..9c8d701d21 100644 --- a/server/serverHelpers/contentTypes.ts +++ b/server/serverHelpers/contentTypes.ts @@ -44,10 +44,18 @@ function streamErrorRecord(error: any) { if (typeof error?.message === 'string') message = error.message; } catch {} message ??= harperErrorToString(error); - const record: { error: string; message: string; status?: number } = { error: name, message }; + const record: { error: string; message: string; status?: number; replicationUnconfirmed?: true; retryable?: false } = + { + error: name, + message, + }; try { const status = error?.statusCode ?? error?.status; if (typeof status === 'number') record.status = status; + if (error?.replicationUnconfirmed === true) { + record.replicationUnconfirmed = true; + record.retryable = false; + } } catch {} return record; } diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index 038e1376bc..dc95a50dda 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -90,22 +90,22 @@ function serverErrorHandler(error, req, resp) { } return resp.code(error.statusCode).send(body); } - return resp - .code(error.statusCode) - .send( - error.replicationUnconfirmed === true - ? { ...error.http_resp_msg, replicationUnconfirmed: true, retryable: false } - : error.http_resp_msg - ); + if (error.replicationUnconfirmed === true) { + const body = Array.isArray(error.http_resp_msg) ? { error: error.http_resp_msg } : error.http_resp_msg; + return resp.code(error.statusCode).send({ + ...body, + code: body?.code ?? error.code, + replicationUnconfirmed: true, + retryable: false, + }); + } + return resp.code(error.statusCode).send(error.http_resp_msg); } const statusCode = error.statusCode ? error.statusCode : hdbErrors.HTTP_STATUS_CODES.INTERNAL_SERVER_ERROR; if (typeof error === 'string') { return resp.code(statusCode).send({ error: error }); } - const body = error.message ? { error: error.message } : error; - return resp - .code(statusCode) - .send(error.replicationUnconfirmed === true ? { ...body, replicationUnconfirmed: true } : body); + return resp.code(statusCode).send(error.message ? { error: error.message } : error); } function reqBodyValidationHandler(req, resp, done) { diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 75380a42ee..2ef95cddd9 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -14,6 +14,7 @@ const { ReplicationConfirmationError, ServerError } = require('#src/utility/erro const { Resource } = require('#src/resources/Resource'); const { Resources } = require('#src/resources/Resources'); const { handleApplication } = require('#src/server/REST'); +const { contentTypes } = require('#src/server/serverHelpers/contentTypes'); const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; const registerConfirmation = isLMDB ? lmdbConfirmation : rocksConfirmation; @@ -309,6 +310,96 @@ describe('Replication confirmation failure after a local commit', () => { assert.strictEqual((await Second.get(id)).id, id); }); + it('removes a nested commit flag when the enclosing callback aborts pending writes', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + await assert.rejects( + transaction({}, async () => { + await Second.put({ id: `outer-${id}` }); + await confirmedWrite(`inner-${id}`); + }), + (error) => { + assert.strictEqual(error.replicationUnconfirmed, undefined); + assert.strictEqual(error.cause.replicationUnconfirmed, true); + assert.strictEqual(error.statusCode, 504); + assert.strictEqual(error.code, 'ServerError'); + assert.strictEqual(error.retryable, false); + return true; + } + ); + assert.strictEqual((await First.get(`inner-${id}`)).id, `inner-${id}`); + assert.equal(await Second.get(`outer-${id}`), null); + }); + + it('preserves the flag when the callback awaited its own failed commit', async () => { + const id = nextId(); + const context = { replicatedConfirmation: 1 }; + registerConfirmation(() => Promise.reject(new ServerError('Own peer timeout', 504))); + await assert.rejects( + transaction(context, async (scope) => { + await First.put({ id }); + requestConfirmation(context); + await scope.commit(); + }), + (error) => error.replicationUnconfirmed === true + ); + assert.strictEqual((await First.get(id)).id, id); + }); + + it('does not defer a foreign confirmation error from local pre-commit work', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + class NestedSource extends Second {} + NestedSource.sourcedFrom({ get() {}, put: () => confirmedWrite(`inner-source-${id}`) }); + await assert.rejects( + transaction({}, () => NestedSource.put({ id: `outer-source-${id}` })), + (error) => { + assert.strictEqual(error.replicationUnconfirmed, undefined); + assert.strictEqual(error.cause.replicationUnconfirmed, true); + return true; + } + ); + assert.strictEqual((await First.get(`inner-source-${id}`)).id, `inner-source-${id}`); + assert.equal(await Second.get(`outer-source-${id}`), null); + }); + + for (const type of ['application/json', 'application/x-ndjson', 'text/event-stream']) { + it(`retains a post-commit confirmation flag in a terminal ${type} record`, async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Stream peer timeout', 504))); + async function* source() { + yield { stage: 'before-write' }; + await confirmedWrite(id); + } + const stream = contentTypes.get(type).serializeStream(source(), undefined, { method: 'GET' }); + const chunks = []; + for await (const chunk of stream) chunks.push(Buffer.from(chunk)); + const text = Buffer.concat(chunks).toString(); + const records = + type === 'application/json' + ? JSON.parse(text) + : type === 'text/event-stream' + ? text + .split('\n') + .filter((line) => line.startsWith('data: ')) + .map((line) => JSON.parse(line.slice(6))) + : text + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + assert.deepStrictEqual(records[0], { stage: 'before-write' }); + const failure = records[1].$harperStreamError ?? records[1]; + assert.strictEqual(failure.replicationUnconfirmed, true); + assert.strictEqual(failure.retryable, false); + if (type === 'application/json') assert.match(failure.error, /^ReplicationConfirmationError:/); + else { + assert.strictEqual(failure.error, 'ServerError'); + assert.strictEqual(failure.status, 504); + } + assert.strictEqual((await First.get(id)).id, id); + }); + } + it('publishes the flag in REST problem details while retaining the original public code', async () => { const id = nextId(); registerConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); @@ -340,6 +431,7 @@ describe('Replication confirmation failure after a local commit', () => { assert.strictEqual(response.status, 504); const body = JSON.parse(response.body); assert.strictEqual(body.replicationUnconfirmed, true); + assert.strictEqual(body.retryable, false); assert.strictEqual(body.code, 'ServerError'); assert.strictEqual(body.title, 'Peer timeout'); assert.strictEqual((await First.get(id)).id, id); diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index aa4a23a7c4..ba7197854f 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -33,6 +33,7 @@ describe('serverErrorHandler', function () { ? new HdbError(new Error('Peer timeout'), { error: 'Peer timeout', peers: ['node-a'], retryable: true }, 504) : new ServerError('Peer timeout', 504); cause.retryable = true; + cause.code = 'PEER_TIMEOUT'; const failure = new ReplicationConfirmationError(cause); app.post('/', async () => { throw failure; @@ -41,6 +42,7 @@ describe('serverErrorHandler', function () { const response = await app.inject({ method: 'POST', url: '/' }); assert.strictEqual(response.statusCode, 504); assert.strictEqual(response.json().replicationUnconfirmed, true); + assert.strictEqual(response.json().code, 'PEER_TIMEOUT'); assert.notEqual(response.json().retryable, true); assert.strictEqual(response.json().error, 'Peer timeout'); if (structured) assert.deepStrictEqual(response.json().peers, ['node-a']); @@ -65,7 +67,32 @@ describe('serverErrorHandler', function () { } }); - it('retains the flag through the real operations insert dispatch and a local commit', async () => { + for (const marked of [false, true]) { + it(`preserves array error details${marked ? ' in a flagged envelope' : ' without a flag'}`, async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + const details = ['Peer timeout']; + const cause = new HdbError(new Error('Peer timeout'), details, 504); + cause.code = 'PEER_TIMEOUT'; + app.post('/', async () => { + throw marked ? new ReplicationConfirmationError(cause) : cause; + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.strictEqual(response.statusCode, 504); + assert.deepStrictEqual( + response.json(), + marked ? { error: details, code: 'PEER_TIMEOUT', replicationUnconfirmed: true, retryable: false } : details + ); + assert.deepStrictEqual(cause.http_resp_msg, details); + } finally { + await app.close(); + } + }); + } + + it('retains the flag through the real operations insert dispatch and a local commit', async function () { + if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') this.skip(); setupTestDBPath(); setMainIsWorker(true); const Records = table({ diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index 28def899b1..0b2b1b1985 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -116,6 +116,21 @@ export class ReplicationConfirmationError extends ServerError { } } +export function withoutReplicationUnconfirmed(error: any): ServerError { + let responseMessage = error.http_resp_msg; + if (responseMessage && typeof responseMessage === 'object' && !Array.isArray(responseMessage)) { + responseMessage = { ...responseMessage, retryable: false }; + delete responseMessage.replicationUnconfirmed; + } + return Object.assign(new ServerError(error.message, error.statusCode), { + cause: error, + code: error.code ?? error.constructor?.name, + http_resp_msg: responseMessage, + detail: error.detail, + retryable: false, + }); +} + /** Retryable admission failure when a derived index cannot meet a write or query lag budget. */ export class DerivedIndexLagError extends ServerError { code: string; From b542130f83460d9b37524e1740e502fb6751fd06 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 13:10:09 -0600 Subject: [PATCH 04/14] Keep nonretryable inner outcomes visible after an outer rollback Preserve negative retry hints in REST, streams and structured array operations errors when the outer scope removes the inner commit flag. Define the flag at the reported commit boundary, and return the observed waiter directly when no local completions remain. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 4 +- resources/DatabaseTransaction.ts | 1 + server/REST.ts | 6 +- server/serverHelpers/JSONStream.ts | 6 +- server/serverHelpers/contentTypes.ts | 2 +- .../replicationConfirmationOutcome.test.js | 170 +++++++++++------- .../serverHelpers/serverErrorHandler.test.js | 28 ++- utility/errors/hdbError.ts | 4 +- 8 files changed, 143 insertions(+), 78 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index e4b8e29e9c..a136970daf 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -1005,7 +1005,9 @@ and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes, -keeping the original error in `cause` and refusing automatic retry because the nested write did land. +keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because +the nested write did land. The flag attests to the reported commit's issued writes, not later application +steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index a073d8e0fa..1e4cdaa21f 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -508,6 +508,7 @@ export function confirmReplicationAfterCommit(callback: () => unknown): Promise< } export function settleCommitCompletions(completions: unknown[], confirmation?: Promise): Promise { + if (completions.length === 0) return confirmation ?? Promise.resolve(); let deferredConfirmationError: ReplicationConfirmationError; const localCompletions = completions.map((completion) => Promise.resolve(completion).catch((error) => { diff --git a/server/REST.ts b/server/REST.ts index bf15eeaf22..03ca7b5ef5 100644 --- a/server/REST.ts +++ b/server/REST.ts @@ -480,7 +480,11 @@ async function http(request: Request, nextHandler, resources: Resources, httpOpt status, detail: error.detail, instance: request.url, - ...(error.replicationUnconfirmed === true ? { replicationUnconfirmed: true, retryable: false } : {}), + ...(error.replicationUnconfirmed === true + ? { replicationUnconfirmed: true, retryable: false } + : error.retryable === false + ? { retryable: false } + : {}), }; const responseObject = { diff --git a/server/serverHelpers/JSONStream.ts b/server/serverHelpers/JSONStream.ts index e18ef8960f..606eb51eab 100644 --- a/server/serverHelpers/JSONStream.ts +++ b/server/serverHelpers/JSONStream.ts @@ -12,7 +12,11 @@ const { errorToString } = harperLogger; function streamError(error) { return { error: errorToString(error), - ...(error?.replicationUnconfirmed === true ? { replicationUnconfirmed: true, retryable: false } : {}), + ...(error?.replicationUnconfirmed === true + ? { replicationUnconfirmed: true, retryable: false } + : error?.retryable === false + ? { retryable: false } + : {}), }; } export function streamAsJSON(value) { diff --git a/server/serverHelpers/contentTypes.ts b/server/serverHelpers/contentTypes.ts index 9c8d701d21..ef914360b8 100644 --- a/server/serverHelpers/contentTypes.ts +++ b/server/serverHelpers/contentTypes.ts @@ -55,7 +55,7 @@ function streamErrorRecord(error: any) { if (error?.replicationUnconfirmed === true) { record.replicationUnconfirmed = true; record.retryable = false; - } + } else if (error?.retryable === false) record.retryable = false; } catch {} return record; } diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 2ef95cddd9..87398ec89f 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -331,6 +331,23 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`outer-${id}`), null); }); + it('retains a nested commit outcome before any later application step was issued', async () => { + const id = nextId(); + let laterStepRan = false; + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + await assert.rejects( + transaction({}, async () => { + await confirmedWrite(id); + laterStepRan = true; + await Second.put({ id: `later-${id}` }); + }), + (error) => error.replicationUnconfirmed === true && error.retryable === false + ); + assert.strictEqual(laterStepRan, false); + assert.strictEqual((await First.get(id)).id, id); + assert.equal(await Second.get(`later-${id}`), null); + }); + it('preserves the flag when the callback awaited its own failed commit', async () => { const id = nextId(); const context = { replicatedConfirmation: 1 }; @@ -363,77 +380,94 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`outer-source-${id}`), null); }); - for (const type of ['application/json', 'application/x-ndjson', 'text/event-stream']) { - it(`retains a post-commit confirmation flag in a terminal ${type} record`, async () => { + for (const rollback of [false, true]) { + for (const type of ['application/json', 'application/x-ndjson', 'text/event-stream']) { + it(`retains a post-commit confirmation flag in a terminal ${type} record${rollback ? ' after outer rollback' : ''}`, async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Stream peer timeout', 504))); + async function* source() { + yield { stage: 'before-write' }; + if (rollback) { + await transaction({}, async () => { + await Second.put({ id: `outer-stream-${id}` }); + await confirmedWrite(id); + }); + } else await confirmedWrite(id); + } + const stream = contentTypes.get(type).serializeStream(source(), undefined, { method: 'GET' }); + const chunks = []; + for await (const chunk of stream) chunks.push(Buffer.from(chunk)); + const text = Buffer.concat(chunks).toString(); + const records = + type === 'application/json' + ? JSON.parse(text) + : type === 'text/event-stream' + ? text + .split('\n') + .filter((line) => line.startsWith('data: ')) + .map((line) => JSON.parse(line.slice(6))) + : text + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + assert.deepStrictEqual(records[0], { stage: 'before-write' }); + const failure = records[1].$harperStreamError ?? records[1]; + assert.strictEqual(failure.replicationUnconfirmed, rollback ? undefined : true); + assert.strictEqual(failure.retryable, false); + if (type === 'application/json') + assert.match(failure.error, rollback ? /^ServerError:/ : /^ReplicationConfirmationError:/); + else { + assert.strictEqual(failure.error, 'ServerError'); + assert.strictEqual(failure.status, 504); + } + assert.strictEqual((await First.get(id)).id, id); + if (rollback) assert.equal(await Second.get(`outer-stream-${id}`), null); + }); + } + } + + for (const rollback of [false, true]) { + it(`publishes the REST outcome${rollback ? ' after outer rollback' : ''} while retaining the original public code`, async () => { const id = nextId(); - registerConfirmation(() => Promise.reject(new ServerError('Stream peer timeout', 504))); - async function* source() { - yield { stage: 'before-write' }; - await confirmedWrite(id); - } - const stream = contentTypes.get(type).serializeStream(source(), undefined, { method: 'GET' }); - const chunks = []; - for await (const chunk of stream) chunks.push(Buffer.from(chunk)); - const text = Buffer.concat(chunks).toString(); - const records = - type === 'application/json' - ? JSON.parse(text) - : type === 'text/event-stream' - ? text - .split('\n') - .filter((line) => line.startsWith('data: ')) - .map((line) => JSON.parse(line.slice(6))) - : text - .trim() - .split('\n') - .map((line) => JSON.parse(line)); - assert.deepStrictEqual(records[0], { stage: 'before-write' }); - const failure = records[1].$harperStreamError ?? records[1]; - assert.strictEqual(failure.replicationUnconfirmed, true); - assert.strictEqual(failure.retryable, false); - if (type === 'application/json') assert.match(failure.error, /^ReplicationConfirmationError:/); - else { - assert.strictEqual(failure.error, 'ServerError'); - assert.strictEqual(failure.status, 504); + registerConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); + class Signup extends Resource { + static loadAsInstance = false; + async post(target) { + if (rollback) { + await Second.put({ id: `outer-rest-${target.id}` }); + await confirmedWrite(target.id); + } else { + await First.put({ id: target.id }); + requestConfirmation(contextStorage.getStore()); + } + return 'created'; + } } + const resources = new Resources(); + resources.set('Signup', Signup); + let listener; + handleApplication({ + resources, + options: { getAll: () => ({ webSocket: false }) }, + routeFor: () => ({ urlPath: `/confirmation-outcome-${rollback}` }), + server: { http: (handler) => (listener = handler) }, + }); + const response = await listener({ + method: 'POST', + url: `/Signup/${id}`, + pathname: `/Signup/${id}`, + ip: '203.0.113.7', + user: { username: 'admin', role: { permission: { super_user: true } } }, + headers: { asObject: {}, get: () => undefined }, + }); + assert.strictEqual(response.status, 504); + const body = JSON.parse(response.body); + assert.strictEqual(body.replicationUnconfirmed, rollback ? undefined : true); + assert.strictEqual(body.retryable, false); + assert.strictEqual(body.code, 'ServerError'); + assert.strictEqual(body.title, 'Peer timeout'); assert.strictEqual((await First.get(id)).id, id); + if (rollback) assert.equal(await Second.get(`outer-rest-${id}`), null); }); } - - it('publishes the flag in REST problem details while retaining the original public code', async () => { - const id = nextId(); - registerConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); - class Signup extends Resource { - static loadAsInstance = false; - async post(target) { - await First.put({ id: target.id }); - requestConfirmation(contextStorage.getStore()); - return 'created'; - } - } - const resources = new Resources(); - resources.set('Signup', Signup); - let listener; - handleApplication({ - resources, - options: { getAll: () => ({ webSocket: false }) }, - routeFor: () => ({ urlPath: '/confirmation-outcome' }), - server: { http: (handler) => (listener = handler) }, - }); - const response = await listener({ - method: 'POST', - url: `/Signup/${id}`, - pathname: `/Signup/${id}`, - ip: '203.0.113.7', - user: { username: 'admin', role: { permission: { super_user: true } } }, - headers: { asObject: {}, get: () => undefined }, - }); - assert.strictEqual(response.status, 504); - const body = JSON.parse(response.body); - assert.strictEqual(body.replicationUnconfirmed, true); - assert.strictEqual(body.retryable, false); - assert.strictEqual(body.code, 'ServerError'); - assert.strictEqual(body.title, 'Peer timeout'); - assert.strictEqual((await First.get(id)).id, id); - }); }); diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index ba7197854f..4e13436e8d 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -2,7 +2,13 @@ const assert = require('assert'); const fastify = require('fastify'); -const { hdbErrors, ReplicationConfirmationError, ServerError, HdbError } = require('#src/utility/errors/hdbError'); +const { + hdbErrors, + ReplicationConfirmationError, + ServerError, + HdbError, + withoutReplicationUnconfirmed, +} = require('#src/utility/errors/hdbError'); const { serverErrorHandler, handlePostRequest } = require('#js/server/serverHelpers/serverHandlers'); const { setupTestDBPath } = require('../../testUtils'); const { table } = require('#src/resources/databases'); @@ -67,22 +73,34 @@ describe('serverErrorHandler', function () { } }); - for (const marked of [false, true]) { - it(`preserves array error details${marked ? ' in a flagged envelope' : ' without a flag'}`, async () => { + for (const outcome of ['plain', 'committed', 'stripped']) { + it(`preserves array error details${outcome === 'plain' ? ' without a flag' : ` in a ${outcome} envelope`}`, async () => { const app = fastify(); app.setErrorHandler(serverErrorHandler); const details = ['Peer timeout']; const cause = new HdbError(new Error('Peer timeout'), details, 504); cause.code = 'PEER_TIMEOUT'; app.post('/', async () => { - throw marked ? new ReplicationConfirmationError(cause) : cause; + const committed = new ReplicationConfirmationError(cause); + throw outcome === 'plain' + ? cause + : outcome === 'stripped' + ? withoutReplicationUnconfirmed(committed) + : committed; }); try { const response = await app.inject({ method: 'POST', url: '/' }); assert.strictEqual(response.statusCode, 504); assert.deepStrictEqual( response.json(), - marked ? { error: details, code: 'PEER_TIMEOUT', replicationUnconfirmed: true, retryable: false } : details + outcome === 'plain' + ? details + : { + error: details, + code: 'PEER_TIMEOUT', + retryable: false, + ...(outcome === 'committed' ? { replicationUnconfirmed: true } : {}), + } ); assert.deepStrictEqual(cause.http_resp_msg, details); } finally { diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index 0b2b1b1985..c2c88d4637 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -118,7 +118,9 @@ export class ReplicationConfirmationError extends ServerError { export function withoutReplicationUnconfirmed(error: any): ServerError { let responseMessage = error.http_resp_msg; - if (responseMessage && typeof responseMessage === 'object' && !Array.isArray(responseMessage)) { + if (Array.isArray(responseMessage)) { + responseMessage = { error: responseMessage, code: error.code, retryable: false }; + } else if (responseMessage && typeof responseMessage === 'object') { responseMessage = { ...responseMessage, retryable: false }; delete responseMessage.replicationUnconfirmed; } From 14541da2dabd479902dd2d2f912ad23066ade516 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 15:05:55 -0600 Subject: [PATCH 05/14] Finish JSON error streams even when outcome metadata is unreadable Guard extension getters on arbitrary iterator rejection reasons so terminal JSON remains valid. Document the terminal metadata and point confirmation-hook ordering to its native-store regression suite. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 5 +-- server/DESIGN.md | 7 +++- server/serverHelpers/JSONStream.ts | 17 +++++----- .../server/serverHelpers/contentTypes.test.js | 33 +++++++++++++++++++ 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index a136970daf..93e292ac1a 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -987,8 +987,9 @@ the recipient bound). A table source's `before` hook (`source.put`/`patch`) has record-based placement needs the merged record; the guarantee covers Harper's stored data, not the source. A confirmation-hook error after the native commit is observed after local completions, so bookkeeping and chained commits finish before a confirmation failure surfaces. LMDB's production registry is unregistered and its write-bearing -child does not inherit the root's confirmation count. Enforced by -`unitTests/resources/residencyConfirmation.test.js`. +child does not inherit the root's confirmation count. Feasibility and residency are pinned by +`unitTests/resources/residencyConfirmation.test.js`; post-hook ordering is pinned by +`unitTests/resources/replicationConfirmationOutcome.test.js`. ## Replication confirmation failure means the linked writes committed locally diff --git a/server/DESIGN.md b/server/DESIGN.md index b6f786222d..e925649797 100644 --- a/server/DESIGN.md +++ b/server/DESIGN.md @@ -226,7 +226,12 @@ SSE and NDJSON serializers eagerly take and hold their first iterator step for G SSE terminal event data uses `{ error: , message: , status?: }`; NDJSON wraps the same object as `{ "$harperStreamError": { ... } }` so it cannot be mistaken for an ordinary row with an `error` field. Sources with `mapError` remain unmapped for these two formats so their failures reach this single contract instead of becoming legacy data records. -generic JSON-array streaming retains its older `{ error: ": " }` element shape. +Generic JSON-array streaming retains its `{ error: ": " }` element shape. +All three terminal records add `replicationUnconfirmed: true` for a reported commit whose local writes landed +but whose confirmation failed, and `retryable: false` for that outcome or another explicitly non-retryable error. +An enclosing rollback removes the confirmation flag while preserving the negative retry hint. Metadata getters +on a thrown value may fail; omit unreadable extensions and still finish a valid error record. The commit-level +scope and unchanged producer coverage are described in [resources/DESIGN.md](../resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally). The `error` value is the stable programmatic discriminator; `message` is diagnostic and follows the same thrown-message exposure policy as pre-commit Problem Details. Clean stream completion does not prove completeness; clients must inspect SSE for the `harper-error` event and NDJSON for the `$harperStreamError` control record. diff --git a/server/serverHelpers/JSONStream.ts b/server/serverHelpers/JSONStream.ts index 606eb51eab..18b64e8db6 100644 --- a/server/serverHelpers/JSONStream.ts +++ b/server/serverHelpers/JSONStream.ts @@ -10,15 +10,16 @@ const BIGINT_SERIALIZATION = { message: 'Cannot serialize BigInt to JSON' }; }; const { errorToString } = harperLogger; function streamError(error) { - return { - error: errorToString(error), - ...(error?.replicationUnconfirmed === true - ? { replicationUnconfirmed: true, retryable: false } - : error?.retryable === false - ? { retryable: false } - : {}), - }; + const record: { error: string; replicationUnconfirmed?: true; retryable?: false } = { error: errorToString(error) }; + try { + if (error?.replicationUnconfirmed === true) { + record.replicationUnconfirmed = true; + record.retryable = false; + } else if (error?.retryable === false) record.retryable = false; + } catch {} + return record; } + export function streamAsJSON(value) { return new JSONStream({ value }); } diff --git a/unitTests/server/serverHelpers/contentTypes.test.js b/unitTests/server/serverHelpers/contentTypes.test.js index 07b4539d7a..16a0e159db 100644 --- a/unitTests/server/serverHelpers/contentTypes.test.js +++ b/unitTests/server/serverHelpers/contentTypes.test.js @@ -854,3 +854,36 @@ describe('contentTypes – a multipart upload its route answers without reading' } }); }); + +describe('contentTypes – JSON terminal metadata', () => { + for (const asynchronous of [false, true]) { + for (const property of ['replicationUnconfirmed', 'retryable']) { + it(`finishes valid JSON when ${property} throws in a ${asynchronous ? 'async' : 'sync'} iterator error`, async function () { + this.timeout(2000); + const failure = new Proxy(new Error('Iterator failed'), { + get(target, key, receiver) { + if (key === property) throw new Error('Metadata is unreadable'); + return Reflect.get(target, key, receiver); + }, + }); + function* syncSource() { + yield { id: 'first' }; + throw failure; + } + async function* asyncSource() { + yield { id: 'first' }; + throw failure; + } + const readable = contentTypes + .get('application/json') + .serializeStream(asynchronous ? asyncSource() : syncSource()); + try { + const result = JSON.parse(await streamToString(readable)); + assert.deepStrictEqual(result, [{ id: 'first' }, { error: 'Error: Iterator failed' }]); + } finally { + readable.destroy(); + } + }); + } + } +}); From d9b3d0c1f4d0c64212823df14a78a03241aa7f34 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 16:48:23 -0600 Subject: [PATCH 06/14] Keep transaction cleanup and confirmation outcomes safe from unreadable metadata Guard error inspection before aborting, preserve the committed outcome when cause diagnostics throw, and drop unreadable structured details before removing a nested flag. Pin callback/source rollback and confirmation diagnostics against real stores on both engines. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 2 + resources/DatabaseTransaction.ts | 7 +- resources/LMDBTransaction.ts | 5 +- resources/transaction.ts | 14 +-- server/DESIGN.md | 3 +- .../replicationConfirmationOutcome.test.js | 95 +++++++++++++++++++ utility/errors/hdbError.ts | 37 +++++--- 7 files changed, 141 insertions(+), 22 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 93e292ac1a..e85cdcdcb7 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -1013,6 +1013,8 @@ steps that never ran after a rejection; a nested flag can remain when there are Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) render the new class name. Object responses preserve the original public code alongside the flag. +Unreadable error properties are omitted rather than obscuring the commit outcome or preventing rollback; +unreadable structured details fall back to the ordinary error response when an outer rollback strips the flag. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 1e4cdaa21f..7188576caa 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -3022,7 +3022,7 @@ export class DatabaseTransaction implements Transaction { } } catch (error) { const refusal = Promise.reject(error); - refusal.catch(() => {}); // surfaces through Promise.all(completions), even if bookkeeping throws first + refusal.catch(() => {}); // surfaces through local completions, even if bookkeeping throws first completions.push(refusal); } } @@ -3379,7 +3379,10 @@ export class DatabaseTransaction implements Transaction { } protected abortAfterCommitError(error, targetTransaction?: RocksTransaction): never { - if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) error = withoutReplicationUnconfirmed(error); + try { + if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) + error = withoutReplicationUnconfirmed(error); + } catch {} try { const retryTxn = targetTransaction as RocksTransactionWithRetry | undefined; const retryTxnCommitted = this.isHandleCommitted(retryTxn); diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index 025d3cfe6a..9a4d5e51f3 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -401,7 +401,10 @@ export class LMDBTransaction extends DatabaseTransaction { } private abortCommitFailure(error, next: DatabaseTransaction | undefined): never { - if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) error = withoutReplicationUnconfirmed(error); + try { + if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) + error = withoutReplicationUnconfirmed(error); + } catch {} const nextWasAttached = Boolean(next && this.next === next); try { this.abort(true); diff --git a/resources/transaction.ts b/resources/transaction.ts index 9542093503..db9d00238a 100644 --- a/resources/transaction.ts +++ b/resources/transaction.ts @@ -105,12 +105,14 @@ export function transaction( abortAndThrow(error, true); } function abortAndThrow(error, callbackThrew: boolean): never { - if ( - callbackThrew && - error?.replicationUnconfirmed === true && - (transaction.hasPendingWrites() || transaction.isChainCommitting()) - ) - error = withoutReplicationUnconfirmed(error); + try { + if ( + callbackThrew && + error?.replicationUnconfirmed === true && + (transaction.hasPendingWrites() || transaction.isChainCommitting()) + ) + error = withoutReplicationUnconfirmed(error); + } catch {} // A commit attempt that has not reached its native outcome owns its own teardown — a handler that // fired txn.commit() without awaiting it can get here while it is still running, and aborting // would clear the writes it is committing and abort the handle it is committing them through. diff --git a/server/DESIGN.md b/server/DESIGN.md index e925649797..140fd25248 100644 --- a/server/DESIGN.md +++ b/server/DESIGN.md @@ -227,9 +227,10 @@ SSE and NDJSON serializers eagerly take and hold their first iterator step for G SSE terminal event data uses `{ error: , message: , status?: }`; NDJSON wraps the same object as `{ "$harperStreamError": { ... } }` so it cannot be mistaken for an ordinary row with an `error` field. Sources with `mapError` remain unmapped for these two formats so their failures reach this single contract instead of becoming legacy data records. Generic JSON-array streaming retains its `{ error: ": " }` element shape. +REST problem details publish only an explicitly negative `retryable: false` hint; a missing hint is unknown. All three terminal records add `replicationUnconfirmed: true` for a reported commit whose local writes landed but whose confirmation failed, and `retryable: false` for that outcome or another explicitly non-retryable error. -An enclosing rollback removes the confirmation flag while preserving the negative retry hint. Metadata getters +An enclosing rollback of pending writes removes the confirmation flag while preserving the negative retry hint. Metadata getters on a thrown value may fail; omit unreadable extensions and still finish a valid error record. The commit-level scope and unchanged producer coverage are described in [resources/DESIGN.md](../resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally). The `error` value is the stable programmatic discriminator; `message` is diagnostic and follows the same thrown-message exposure policy as pre-commit Problem Details. diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 87398ec89f..8101d9dfa2 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -102,6 +102,27 @@ describe('Replication confirmation failure after a local commit', () => { assert.strictEqual((await First.get(id)).id, id); }); + for (const property of ['message', 'statusCode', 'code', 'constructor', 'http_resp_msg', 'detail']) { + it(`retains the committed outcome when the confirmation error's ${property} getter throws`, async () => { + const id = nextId(); + const failure = new ServerError('Peer failure', 504); + Object.defineProperty(failure, property, { + get() { + throw new Error('Error metadata is unreadable'); + }, + }); + registerConfirmation(() => Promise.reject(failure)); + await assert.rejects(confirmedWrite(id), (error) => { + assert.ok(error instanceof ReplicationConfirmationError); + assert.strictEqual(error.cause, failure); + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual(error.retryable, false); + return true; + }); + assert.strictEqual((await First.get(id)).id, id); + }); + } + it('preserves the public code and status of an error from another JavaScript realm', async () => { const failure = runInNewContext(` class ForeignConfirmationError extends Error { @@ -137,6 +158,54 @@ describe('Replication confirmation failure after a local commit', () => { ); }); + for (const sourceFailure of [false, true]) { + it(`rolls back when outcome metadata throws in ${sourceFailure ? 'pre-commit source work' : 'the callback'}`, async () => { + const id = nextId(); + const failure = new Error('Application failure'); + Object.defineProperty(failure, 'replicationUnconfirmed', { + get() { + throw new Error('Outcome metadata is unreadable'); + }, + }); + const context = {}; + const links = []; + function rememberLinks() { + for (let link = context.transaction; link; link = link.next) { + if (!links.includes(link)) links.push(link); + } + } + class FailingSource extends First {} + FailingSource.sourcedFrom({ + get() {}, + put() { + rememberLinks(); + throw failure; + }, + }); + try { + await assert.rejects( + transaction(context, async () => { + await First.put({ id }); + rememberLinks(); + await (sourceFailure ? FailingSource : Second).put({ id: sourceFailure ? `source-${id}` : id }); + rememberLinks(); + if (!sourceFailure) throw failure; + }), + (error) => error === failure + ); + for (const link of links) { + assert.strictEqual(link.hasPendingWrites(), false); + assert.ok(!link.transaction); + assert.ok(!link.readTxn); + } + assert.equal(await First.get(id), null); + assert.equal(await (sourceFailure ? First : Second).get(sourceFailure ? `source-${id}` : id), null); + } finally { + context.transaction?.abort(); + } + }); + } + it('retains successful confirmation and the existing callback arguments', async () => { const id = nextId(); const calls = []; @@ -331,6 +400,32 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`outer-${id}`), null); }); + it('removes a nested outcome flag even when structured error details cannot be copied', async () => { + const id = nextId(); + const failure = new ServerError('Nested peer timeout', 504); + failure.http_resp_msg = { + get detail() { + throw new Error('Details are unreadable'); + }, + }; + registerConfirmation(() => Promise.reject(failure)); + await assert.rejects( + transaction({}, async () => { + await Second.put({ id: `outer-${id}` }); + await confirmedWrite(`inner-${id}`); + }), + (error) => { + assert.strictEqual(error.replicationUnconfirmed, undefined); + assert.strictEqual(error.cause.replicationUnconfirmed, true); + assert.strictEqual(error.retryable, false); + assert.strictEqual(error.http_resp_msg, undefined); + return true; + } + ); + assert.strictEqual((await First.get(`inner-${id}`)).id, `inner-${id}`); + assert.equal(await Second.get(`outer-${id}`), null); + }); + it('retains a nested commit outcome before any later application step was issued', async () => { const id = nextId(); let laterStepRan = false; diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index c2c88d4637..aaeb9d8c5d 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -93,6 +93,12 @@ export class ServerError extends Error { } } +function readErrorProperty(error: any, property: string) { + try { + return error?.[property]; + } catch {} +} + export class ReplicationConfirmationError extends ServerError { replicationUnconfirmed = true; retryable = false; @@ -101,28 +107,35 @@ export class ReplicationConfirmationError extends ServerError { detail: unknown; constructor(cause: any) { + const message = readErrorProperty(cause, 'message'); super( - typeof cause?.message === 'string' - ? cause.message - : 'Replication confirmation failed after the local write committed', - cause?.statusCode ?? cause?.status + typeof message === 'string' ? message : 'Replication confirmation failed after the local write committed', + readErrorProperty(cause, 'statusCode') ?? readErrorProperty(cause, 'status') ); this.name = 'ReplicationConfirmationError'; this.cause = cause; this.code = - cause?.code ?? (cause !== null && typeof cause === 'object' ? cause.constructor?.name : undefined) ?? this.name; - this.http_resp_msg = cause?.http_resp_msg; - this.detail = cause?.detail; + readErrorProperty(cause, 'code') ?? + (cause !== null && typeof cause === 'object' + ? readErrorProperty(readErrorProperty(cause, 'constructor'), 'name') + : undefined) ?? + this.name; + this.http_resp_msg = readErrorProperty(cause, 'http_resp_msg'); + this.detail = readErrorProperty(cause, 'detail'); } } export function withoutReplicationUnconfirmed(error: any): ServerError { let responseMessage = error.http_resp_msg; - if (Array.isArray(responseMessage)) { - responseMessage = { error: responseMessage, code: error.code, retryable: false }; - } else if (responseMessage && typeof responseMessage === 'object') { - responseMessage = { ...responseMessage, retryable: false }; - delete responseMessage.replicationUnconfirmed; + try { + if (Array.isArray(responseMessage)) { + responseMessage = { error: responseMessage, code: error.code, retryable: false }; + } else if (responseMessage && typeof responseMessage === 'object') { + responseMessage = { ...responseMessage, retryable: false }; + delete responseMessage.replicationUnconfirmed; + } + } catch { + responseMessage = undefined; } return Object.assign(new ServerError(error.message, error.statusCode), { cause: error, From 80b4a418fb84ddbe3acc2a3f0d2f1d03f7ca75a1 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 19:10:48 -0600 Subject: [PATCH 07/14] Preserve commit outcome fields when diagnostic accessors fail Use guarded diagnostic reads when stripping a nested outcome, guard the new HTTP extensions, and retain the confirmed outcome when structured operations details cannot be copied. Verify real native writes and rollback, both HTTP serializers, non-string diagnostics, and the no-pending-write control. Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 9 +- server/REST.ts | 15 ++- server/serverHelpers/serverHandlers.js | 29 +++-- .../replicationConfirmationOutcome.test.js | 112 ++++++++++++++++++ .../serverHelpers/serverErrorHandler.test.js | 110 +++++++++++------ utility/errors/hdbError.ts | 26 ++-- 6 files changed, 242 insertions(+), 59 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index e85cdcdcb7..a98a126cad 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -995,7 +995,8 @@ child does not inherit the root's confirmation count. Feasibility and residency `ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` -waits for local work first, deferring even a child's confirmation failure. Local write/flush failures +waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, +is deferred too). Local write/flush failures remain prompt and unmarked. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, @@ -1013,8 +1014,10 @@ steps that never ran after a rejection; a nested flag can remain when there are Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) render the new class name. Object responses preserve the original public code alongside the flag. -Unreadable error properties are omitted rather than obscuring the commit outcome or preventing rollback; -unreadable structured details fall back to the ordinary error response when an outer rollback strips the flag. +Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. +Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the new +outcome extensions and structured confirmation details; existing top-level diagnostic reads in HTTP error renderers +are not hardened for arbitrary application errors. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) diff --git a/server/REST.ts b/server/REST.ts index 03ca7b5ef5..623224565a 100644 --- a/server/REST.ts +++ b/server/REST.ts @@ -473,6 +473,15 @@ async function http(request: Request, nextHandler, resources: Resources, httpOpt // we prefer to use error classes for error codes (constructor.name), but if there is a code, it is probably a node.js // error that denotes error codes with a separate property const code = error.code ?? error.constructor.name; + let outcomeExtensions; + try { + outcomeExtensions = + error.replicationUnconfirmed === true + ? { replicationUnconfirmed: true, retryable: false } + : error.retryable === false + ? { retryable: false } + : undefined; + } catch {} const problemDetail = { type: `error:${code}`, // eventually we want this to be a resolvable URI to our docs code, @@ -480,11 +489,7 @@ async function http(request: Request, nextHandler, resources: Resources, httpOpt status, detail: error.detail, instance: request.url, - ...(error.replicationUnconfirmed === true - ? { replicationUnconfirmed: true, retryable: false } - : error.retryable === false - ? { retryable: false } - : {}), + ...outcomeExtensions, }; const responseObject = { diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index dc95a50dda..04ce9c06b1 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -78,23 +78,38 @@ function serverErrorHandler(error, req, resp) { // Fastify passes a handler's reason through as-is, so a rejection with none arrives here as nullish. error ??= handleHDBError(new Error('The request failed without an error'), undefined, 500); if (!errorsLoggedByRequest.get(req)?.has(error)) harperLogger[error.logLevel || 'info'](error); + let replicationUnconfirmed = false; + try { + replicationUnconfirmed = error.replicationUnconfirmed === true; + } catch {} if (error.statusCode) { if (typeof error.http_resp_msg !== 'object') { const body = { error: error.http_resp_msg || error.message }; - if (error.replicationUnconfirmed === true) body.replicationUnconfirmed = true; + if (replicationUnconfirmed) body.replicationUnconfirmed = true; // surface the machine-readable signal for errors that declare retryability (e.g. a // still-building index, IndexRebuildingError) so API callers can distinguish and retry - if (error.retryable !== undefined) { - body.code = error.code; - body.retryable = error.retryable; + try { + if (replicationUnconfirmed || error.retryable !== undefined) { + body.retryable = replicationUnconfirmed ? false : error.retryable; + body.code = error.code; + } + } catch { + try { + body.code = error.code; + } catch {} } return resp.code(error.statusCode).send(body); } - if (error.replicationUnconfirmed === true) { - const body = Array.isArray(error.http_resp_msg) ? { error: error.http_resp_msg } : error.http_resp_msg; + if (replicationUnconfirmed) { + let body; + try { + body = Array.isArray(error.http_resp_msg) ? { error: error.http_resp_msg } : { ...error.http_resp_msg }; + body.code ??= error.code; + } catch { + body = { error: error.message, code: error.code }; + } return resp.code(error.statusCode).send({ ...body, - code: body?.code ?? error.code, replicationUnconfirmed: true, retryable: false, }); diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 8101d9dfa2..b454177004 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -426,6 +426,56 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`outer-${id}`), null); }); + for (const property of [ + 'message', + 'statusCode', + 'code', + 'constructor', + 'http_resp_msg', + 'detail', + 'message-symbol', + 'message-object', + ]) { + it(`removes a nested flag with ${property} diagnostics`, async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + let innerError; + await assert.rejects( + transaction({}, async () => { + await Second.put({ id: `outer-${id}` }); + try { + await confirmedWrite(`inner-${id}`); + } catch (error) { + innerError = error; + if (property === 'constructor') error.code = undefined; + if (property === 'message-symbol') error.message = Symbol('unreadable'); + else if (property === 'message-object') + error.message = { + toString() { + throw new Error('Coercion failed'); + }, + }; + else + Object.defineProperty(error, property, { + get() { + throw new Error('Diagnostic is unreadable'); + }, + }); + throw error; + } + }), + (error) => { + assert.strictEqual(error.replicationUnconfirmed, undefined); + assert.strictEqual(error.retryable, false); + assert.strictEqual(error.cause, innerError); + return true; + } + ); + assert.strictEqual((await First.get(`inner-${id}`)).id, `inner-${id}`); + assert.equal(await Second.get(`outer-${id}`), null); + }); + } + it('retains a nested commit outcome before any later application step was issued', async () => { const id = nextId(); let laterStepRan = false; @@ -443,6 +493,29 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`later-${id}`), null); }); + it('keeps an inner flag with unreadable diagnostics when there are no outer writes to abort', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + let innerError; + await assert.rejects( + transaction({}, async () => { + try { + await confirmedWrite(id); + } catch (error) { + innerError = error; + Object.defineProperty(error, 'message', { + get() { + throw new Error('Diagnostic is unreadable'); + }, + }); + throw error; + } + }), + (error) => error === innerError && error.replicationUnconfirmed === true && error.retryable === false + ); + assert.strictEqual((await First.get(id)).id, id); + }); + it('preserves the flag when the callback awaited its own failed commit', async () => { const id = nextId(); const context = { replicatedConfirmation: 1 }; @@ -521,6 +594,45 @@ describe('Replication confirmation failure after a local commit', () => { } } + for (const property of ['replicationUnconfirmed', 'retryable']) { + it(`retains a REST error response when ${property} cannot be read`, async () => { + const failure = new ServerError('Diagnostic failure', 504); + failure.code = 'DIAGNOSTIC_FAILURE'; + Object.defineProperty(failure, property, { + get() { + throw new Error('Outcome metadata is unreadable'); + }, + }); + class Diagnostic extends Resource { + static loadAsInstance = false; + async post() { + throw failure; + } + } + const resources = new Resources(); + resources.set('Diagnostic', Diagnostic); + let listener; + handleApplication({ + resources, + options: { getAll: () => ({ webSocket: false }) }, + routeFor: () => ({ urlPath: `/diagnostic-outcome-${property}` }), + server: { http: (handler) => (listener = handler) }, + }); + const response = await listener({ + method: 'POST', + url: '/Diagnostic/error', + pathname: '/Diagnostic/error', + ip: '203.0.113.7', + user: { username: 'admin', role: { permission: { super_user: true } } }, + headers: { asObject: {}, get: () => undefined }, + }); + assert.strictEqual(response.status, 504); + assert.strictEqual(JSON.parse(response.body).title, 'Diagnostic failure'); + assert.strictEqual(JSON.parse(response.body).code, 'DIAGNOSTIC_FAILURE'); + assert.strictEqual(JSON.parse(response.body).replicationUnconfirmed, undefined); + }); + } + for (const rollback of [false, true]) { it(`publishes the REST outcome${rollback ? ' after outer rollback' : ''} while retaining the original public code`, async () => { const id = nextId(); diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index 4e13436e8d..c1850d463f 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -73,6 +73,33 @@ describe('serverErrorHandler', function () { } }); + for (const property of ['replicationUnconfirmed', 'retryable']) { + it(`retains an operations error response when ${property} cannot be read`, async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + const failure = new ServerError('Diagnostic failure', 504); + failure.code = 'DIAGNOSTIC_FAILURE'; + failure.retryable = false; + Object.defineProperty(failure, property, { + get() { + throw new Error('Outcome metadata is unreadable'); + }, + }); + app.post('/', async () => { + throw failure; + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.strictEqual(response.statusCode, 504); + assert.strictEqual(response.json().error, 'Diagnostic failure'); + assert.strictEqual(response.json().code, 'DIAGNOSTIC_FAILURE'); + assert.strictEqual(response.json().replicationUnconfirmed, undefined); + } finally { + await app.close(); + } + }); + } + for (const outcome of ['plain', 'committed', 'stripped']) { it(`preserves array error details${outcome === 'plain' ? ' without a flag' : ` in a ${outcome} envelope`}`, async () => { const app = fastify(); @@ -109,40 +136,53 @@ describe('serverErrorHandler', function () { }); } - it('retains the flag through the real operations insert dispatch and a local commit', async function () { - if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') this.skip(); - setupTestDBPath(); - setMainIsWorker(true); - const Records = table({ - database: 'confirmation-outcome-operations', - table: 'Records', - attributes: [{ name: 'id', isPrimaryKey: true }], - }); - const app = fastify(); - app.setErrorHandler(serverErrorHandler); - app.addHook('preHandler', async (request) => { - request.body.hdb_user = { username: 'admin', role: { permission: { super_user: true } } }; - }); - app.post('/', handlePostRequest); - replicationConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); - try { - const response = await app.inject({ - method: 'POST', - url: '/', - payload: { - operation: 'insert', - schema: 'confirmation-outcome-operations', - table: 'Records', - records: [{ id: 'committed' }], - replicatedConfirmation: 1, - }, + for (const unreadable of [false, true]) { + it(`retains the flag through the real operations insert dispatch and a local commit${unreadable ? ' with unreadable structured details' : ''}`, async function () { + if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') this.skip(); + setupTestDBPath(); + setMainIsWorker(true); + const Records = table({ + database: 'confirmation-outcome-operations', + table: 'Records', + attributes: [{ name: 'id', isPrimaryKey: true }], }); - assert.strictEqual(response.statusCode, 504, response.body); - assert.strictEqual(response.json().replicationUnconfirmed, true); - assert.strictEqual((await Records.get('committed')).id, 'committed'); - } finally { - replicationConfirmation(undefined); - await app.close(); - } - }); + const id = unreadable ? 'committed-unreadable' : 'committed'; + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + app.addHook('preHandler', async (request) => { + request.body.hdb_user = { username: 'admin', role: { permission: { super_user: true } } }; + }); + app.post('/', handlePostRequest); + const failure = new ServerError('Peer timeout', 504); + if (unreadable) { + failure.http_resp_msg = { + get detail() { + throw new Error('Details are unreadable'); + }, + }; + } + replicationConfirmation(() => Promise.reject(failure)); + try { + const response = await app.inject({ + method: 'POST', + url: '/', + payload: { + operation: 'insert', + schema: 'confirmation-outcome-operations', + table: 'Records', + records: [{ id }], + replicatedConfirmation: 1, + }, + }); + assert.strictEqual(response.statusCode, 504, response.body); + assert.strictEqual(response.json().replicationUnconfirmed, true); + assert.strictEqual(response.json().retryable, false); + assert.strictEqual(response.json().error, 'Peer timeout'); + assert.strictEqual((await Records.get(id)).id, id); + } finally { + replicationConfirmation(undefined); + await app.close(); + } + }); + } }); diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index aaeb9d8c5d..bdc5bcbe2f 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -126,10 +126,12 @@ export class ReplicationConfirmationError extends ServerError { } export function withoutReplicationUnconfirmed(error: any): ServerError { - let responseMessage = error.http_resp_msg; + const message = readErrorProperty(error, 'message'); + const code = readErrorProperty(error, 'code') ?? readErrorProperty(readErrorProperty(error, 'constructor'), 'name'); + let responseMessage = readErrorProperty(error, 'http_resp_msg'); try { if (Array.isArray(responseMessage)) { - responseMessage = { error: responseMessage, code: error.code, retryable: false }; + responseMessage = { error: responseMessage, code, retryable: false }; } else if (responseMessage && typeof responseMessage === 'object') { responseMessage = { ...responseMessage, retryable: false }; delete responseMessage.replicationUnconfirmed; @@ -137,13 +139,19 @@ export function withoutReplicationUnconfirmed(error: any): ServerError { } catch { responseMessage = undefined; } - return Object.assign(new ServerError(error.message, error.statusCode), { - cause: error, - code: error.code ?? error.constructor?.name, - http_resp_msg: responseMessage, - detail: error.detail, - retryable: false, - }); + return Object.assign( + new ServerError( + typeof message === 'string' ? message : 'Replication confirmation failed', + readErrorProperty(error, 'statusCode') + ), + { + cause: error, + code, + http_resp_msg: responseMessage, + detail: readErrorProperty(error, 'detail'), + retryable: false, + } + ); } /** Retryable admission failure when a derived index cannot meet a write or query lag budget. */ From 8b5b1ee7cfb887e46d391d67022d1fd4b469ee3c Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 20:14:44 -0600 Subject: [PATCH 08/14] Retain readable replication outcomes through serializer fallback faults Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 6 +-- server/serverHelpers/contentTypes.ts | 2 + server/serverHelpers/serverHandlers.js | 8 +++- .../server/serverHelpers/contentTypes.test.js | 42 +++++++++++++++++++ .../serverHelpers/serverErrorHandler.test.js | 35 ++++++++++++++++ 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index a98a126cad..536fd3adb2 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -1013,10 +1013,10 @@ steps that never ran after a rejection; a nested flag can remain when there are Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) -render the new class name. Object responses preserve the original public code alongside the flag. +render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. -Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the new -outcome extensions and structured confirmation details; existing top-level diagnostic reads in HTTP error renderers +Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the +`replicationUnconfirmed`/`retryable` extensions and structured confirmation details; existing top-level diagnostic reads in HTTP error renderers are not hardened for arbitrary application errors. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) diff --git a/server/serverHelpers/contentTypes.ts b/server/serverHelpers/contentTypes.ts index ef914360b8..a3fc70ab02 100644 --- a/server/serverHelpers/contentTypes.ts +++ b/server/serverHelpers/contentTypes.ts @@ -52,6 +52,8 @@ function streamErrorRecord(error: any) { try { const status = error?.statusCode ?? error?.status; if (typeof status === 'number') record.status = status; + } catch {} + try { if (error?.replicationUnconfirmed === true) { record.replicationUnconfirmed = true; record.retryable = false; diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index 04ce9c06b1..a14e4b186b 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -106,7 +106,13 @@ function serverErrorHandler(error, req, resp) { body = Array.isArray(error.http_resp_msg) ? { error: error.http_resp_msg } : { ...error.http_resp_msg }; body.code ??= error.code; } catch { - body = { error: error.message, code: error.code }; + body = { error: 'Replication confirmation failed' }; + try { + if (typeof error.message === 'string') body.error = error.message; + } catch {} + try { + body.code = error.code; + } catch {} } return resp.code(error.statusCode).send({ ...body, diff --git a/unitTests/server/serverHelpers/contentTypes.test.js b/unitTests/server/serverHelpers/contentTypes.test.js index 16a0e159db..fa67d17a76 100644 --- a/unitTests/server/serverHelpers/contentTypes.test.js +++ b/unitTests/server/serverHelpers/contentTypes.test.js @@ -887,3 +887,45 @@ describe('contentTypes – JSON terminal metadata', () => { } } }); + +describe('contentTypes – readable outcome with unreadable status', () => { + for (const type of ['application/x-ndjson', 'text/event-stream']) { + for (const property of ['statusCode', 'status']) { + it(`retains the terminal outcome in ${type} when ${property} throws`, async function () { + this.timeout(2000); + const failure = new Error('Peer timeout'); + failure.replicationUnconfirmed = true; + failure.retryable = false; + Object.defineProperty(failure, property, { + get() { + throw new Error('Status is unreadable'); + }, + }); + async function* source() { + yield { id: 'first' }; + throw failure; + } + const readable = contentTypes.get(type).serializeStream(source()); + try { + const text = await streamToString(readable); + const lines = text.trim().split('\n'); + const record = JSON.parse( + type === 'text/event-stream' + ? lines + .filter((line) => line.startsWith('data: ')) + .at(-1) + .slice(6) + : lines.at(-1) + ); + const outcome = record.$harperStreamError ?? record; + assert.strictEqual(outcome.replicationUnconfirmed, true); + assert.strictEqual(outcome.retryable, false); + assert.strictEqual(outcome.message, 'Peer timeout'); + assert.strictEqual(outcome.status, undefined); + } finally { + readable.destroy(); + } + }); + } + } +}); diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index c1850d463f..9ab882397b 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -136,6 +136,41 @@ describe('serverErrorHandler', function () { }); } + for (const property of ['message', 'code']) { + it(`retains the operations outcome when structured fallback ${property} is unreadable`, async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + const failure = new ReplicationConfirmationError(new ServerError('Peer timeout', 504)); + failure.code = 'PEER_TIMEOUT'; + failure.http_resp_msg = { + get detail() { + throw new Error('Details are unreadable'); + }, + }; + Object.defineProperty(failure, property, { + get() { + throw new Error('Diagnostic is unreadable'); + }, + }); + app.post('/', async () => { + throw failure; + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.strictEqual(response.statusCode, 504, response.body); + assert.strictEqual(response.json().replicationUnconfirmed, true); + assert.strictEqual(response.json().retryable, false); + assert.strictEqual( + response.json().error, + property === 'message' ? 'Replication confirmation failed' : 'Peer timeout' + ); + assert.strictEqual(response.json().code, property === 'code' ? undefined : 'PEER_TIMEOUT'); + } finally { + await app.close(); + } + }); + } + for (const unreadable of [false, true]) { it(`retains the flag through the real operations insert dispatch and a local commit${unreadable ? ' with unreadable structured details' : ''}`, async function () { if (process.env.HARPER_STORAGE_ENGINE === 'lmdb') this.skip(); From 1d445045e116bcd812e77e82ef9796a2226e0fc6 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 20:32:16 -0600 Subject: [PATCH 09/14] Pin the negative retry hint and clarify diagnostic scope Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 4 ++-- unitTests/server/serverHelpers/serverErrorHandler.test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 536fd3adb2..a9ef69da7d 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -1016,8 +1016,8 @@ Programmatic errors are `ReplicationConfirmationError` wrappers, not the origina render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the -`replicationUnconfirmed`/`retryable` extensions and structured confirmation details; existing top-level diagnostic reads in HTTP error renderers -are not hardened for arbitrary application errors. +`replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' +base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index 9ab882397b..dd57b206eb 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -49,7 +49,7 @@ describe('serverErrorHandler', function () { assert.strictEqual(response.statusCode, 504); assert.strictEqual(response.json().replicationUnconfirmed, true); assert.strictEqual(response.json().code, 'PEER_TIMEOUT'); - assert.notEqual(response.json().retryable, true); + assert.strictEqual(response.json().retryable, false); assert.strictEqual(response.json().error, 'Peer timeout'); if (structured) assert.deepStrictEqual(response.json().peers, ['node-a']); assert.strictEqual(cause.replicationUnconfirmed, undefined); From 38e8f47a0bd8573748640accc430938d32651e57 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 20 Oct 2026 08:21:49 -0600 Subject: [PATCH 10/14] Keep confirmation design notes within the directory budget Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 41 ++++------------------------------------- 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index a9ef69da7d..0bdc867384 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -978,46 +978,13 @@ Ids are resolved through a bounded per-database cache of frozen lists (`confirmi list. Count-based placement (`replicateTo: N`) keeps the writer out of the carried-over previous residency, or every update would repeat it and shed a replica. -Every deterministic refusal precedes native submission, so the refused write never lands. A count larger -than a record's list is refused in the commit handler before the write stages anything, and the refused -write turns inert, so a caller that catches the refusal cannot commit it, even on a retry. Before native -submission the commit rechecks the lists (confirmation may have been requested after a write was saved) -and runs the feasibility check the replication layer registered with `replicationConfirmation` (harper-pro: -the recipient bound). A table source's `before` hook (`source.put`/`patch`) has already run by then, because -record-based placement needs the merged record; the guarantee covers Harper's stored data, not the source. -A confirmation-hook error after the native commit is observed after local completions, so bookkeeping and chained commits -finish before a confirmation failure surfaces. LMDB's production registry is unregistered and its write-bearing -child does not inherit the root's confirmation count. Feasibility and residency are pinned by -`unitTests/resources/residencyConfirmation.test.js`; post-hook ordering is pinned by -`unitTests/resources/replicationConfirmationOutcome.test.js`. +Every deterministic refusal precedes native submission, so the refused write never lands. A count larger than a record's list is refused in the commit handler before the write stages anything, and the refused write turns inert, so a caller that catches the refusal cannot commit it, even on a retry. Before native submission the commit rechecks the lists (confirmation may have been requested after a write was saved) and runs the feasibility check the replication layer registered with `replicationConfirmation` (harper-pro: the recipient bound). A table source's `before` hook (`source.put`/`patch`) has already run by then, because record-based placement needs the merged record; the guarantee covers Harper's stored data, not the source. A confirmation-hook error after the native commit is observed after local completions, so bookkeeping and chained commits finish before a confirmation failure surfaces. LMDB's production registry is unregistered and its write-bearing child does not inherit the root's confirmation count. Feasibility and residency are pinned by `unitTests/resources/residencyConfirmation.test.js`; post-hook ordering is pinned by `unitTests/resources/replicationConfirmationOutcome.test.js`. ## Replication confirmation failure means the linked writes committed locally -`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook -failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` -waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, -is deferred too). Local write/flush failures -remain prompt and unmarked. The wrapper retains the original public code, message and status through -`cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat -committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, -the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot -resume writes. Confirmation registration and count propagation are unchanged (LMDB and later databases -are still not automatically confirmed). Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` -and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. - -A confirmation error from an independent nested transaction cannot attest to an enclosing scope's -pending writes: the commit and callback failure funnels remove its flag before aborting those writes, -keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because -the nested write did land. The flag attests to the reported commit's issued writes, not later application -steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. - -Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect -`cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) -render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. -Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. -Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the -`replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' -base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. +`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, is deferred too). Local write/flush failures remain prompt and unmarked. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Confirmation registration and count propagation are unchanged (LMDB and later databases are still not automatically confirmed). Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes, keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because the nested write did land. The flag attests to the reported commit's issued writes, not later application steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. + +Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the `replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) From 48a0a925228477120a4d8007a4ba6371c97df2fc Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 20 Oct 2026 10:08:00 -0600 Subject: [PATCH 11/14] Preserve structured error codes after an enclosing rollback Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 4 ++-- server/DESIGN.md | 2 +- .../serverHelpers/serverErrorHandler.test.js | 19 +++++++++++++++++++ utility/errors/hdbError.ts | 1 + 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 0bdc867384..656f224b4d 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -982,9 +982,9 @@ Every deterministic refusal precedes native submission, so the refused write nev ## Replication confirmation failure means the linked writes committed locally -`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, is deferred too). Local write/flush failures remain prompt and unmarked. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Confirmation registration and count propagation are unchanged (LMDB and later databases are still not automatically confirmed). Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes, keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because the nested write did land. The flag attests to the reported commit's issued writes, not later application steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. +`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, is deferred too). Local write/flush failures reject promptly without the flag. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Only a transaction's root link inherits its context's count; LMDB's write-bearing child and later database links are not automatically confirmed. Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes, keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because the nested write did land. The flag attests to the reported commit's issued writes, not later application steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. -Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-only errors (native HTTP handlers and WebSocket close reasons) render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the `replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. +Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-valued errors (native HTTP handlers, WebSocket close reasons and JSON-array stream elements) render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the `replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. ## This node's short id is 0, confirmed per audit store (`nodeIdMapping.ts` `getThisNodeId`) diff --git a/server/DESIGN.md b/server/DESIGN.md index 140fd25248..f69bea5376 100644 --- a/server/DESIGN.md +++ b/server/DESIGN.md @@ -232,7 +232,7 @@ All three terminal records add `replicationUnconfirmed: true` for a reported com but whose confirmation failed, and `retryable: false` for that outcome or another explicitly non-retryable error. An enclosing rollback of pending writes removes the confirmation flag while preserving the negative retry hint. Metadata getters on a thrown value may fail; omit unreadable extensions and still finish a valid error record. The commit-level -scope and unchanged producer coverage are described in [resources/DESIGN.md](../resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally). +scope and producer coverage are described in [resources/DESIGN.md](../resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally). The `error` value is the stable programmatic discriminator; `message` is diagnostic and follows the same thrown-message exposure policy as pre-commit Problem Details. Clean stream completion does not prove completeness; clients must inspect SSE for the `harper-error` event and NDJSON for the `$harperStreamError` control record. diff --git a/unitTests/server/serverHelpers/serverErrorHandler.test.js b/unitTests/server/serverHelpers/serverErrorHandler.test.js index dd57b206eb..7cb801388c 100644 --- a/unitTests/server/serverHelpers/serverErrorHandler.test.js +++ b/unitTests/server/serverHelpers/serverErrorHandler.test.js @@ -136,6 +136,25 @@ describe('serverErrorHandler', function () { }); } + it('keeps the public code in structured details after stripping the confirmation flag', async () => { + const app = fastify(); + app.setErrorHandler(serverErrorHandler); + const details = { error: 'Peer timeout', peers: ['node-a'] }; + const cause = new HdbError(new Error('Peer timeout'), details, 504); + cause.code = 'PEER_TIMEOUT'; + app.post('/', async () => { + throw withoutReplicationUnconfirmed(new ReplicationConfirmationError(cause)); + }); + try { + const response = await app.inject({ method: 'POST', url: '/' }); + assert.strictEqual(response.statusCode, 504); + assert.deepStrictEqual(response.json(), { ...details, code: 'PEER_TIMEOUT', retryable: false }); + assert.deepStrictEqual(cause.http_resp_msg, details); + } finally { + await app.close(); + } + }); + for (const property of ['message', 'code']) { it(`retains the operations outcome when structured fallback ${property} is unreadable`, async () => { const app = fastify(); diff --git a/utility/errors/hdbError.ts b/utility/errors/hdbError.ts index bdc5bcbe2f..2bd11d9060 100644 --- a/utility/errors/hdbError.ts +++ b/utility/errors/hdbError.ts @@ -134,6 +134,7 @@ export function withoutReplicationUnconfirmed(error: any): ServerError { responseMessage = { error: responseMessage, code, retryable: false }; } else if (responseMessage && typeof responseMessage === 'object') { responseMessage = { ...responseMessage, retryable: false }; + responseMessage.code ??= code; delete responseMessage.replicationUnconfirmed; } } catch { From 226ea836df2740dbada92f4d3c172d44fb3c9ae3 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 20 Oct 2026 10:58:46 -0600 Subject: [PATCH 12/14] Retain rollback history when reporting nested confirmation failures Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DESIGN.md | 2 +- resources/DatabaseTransaction.ts | 24 ++- resources/LMDBTransaction.ts | 5 +- resources/transaction.ts | 2 +- .../replicationConfirmationOutcome.test.js | 193 +++++++++++++++++- 5 files changed, 220 insertions(+), 6 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 656f224b4d..7febe57820 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -982,7 +982,7 @@ Every deterministic refusal precedes native submission, so the refused write nev ## Replication confirmation failure means the linked writes committed locally -`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, is deferred too). Local write/flush failures reject promptly without the flag. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Only a transaction's root link inherits its context's count; LMDB's write-bearing child and later database links are not automatically confirmed. Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes, keeping the original error in `cause` and `retryable: false` in programmatic and HTTP errors because the nested write did land. The flag attests to the reported commit's issued writes, not later application steps that never ran after a rejection; a nested flag can remain when there are no enclosing writes to abort. +`ReplicationConfirmationError.replicationUnconfirmed` is true only for a post-commit confirmation-hook failure. `confirmReplicationAfterCommit` observes the hook immediately, and `settleCommitCompletions` waits for local work first (a child's own confirmation failure, requiring an explicitly assigned link count, is deferred too). Local write/flush failures reject promptly without the flag. The wrapper retains the original public code, message and status through `cause`, without mutating a shared rejection, and advertises `retryable: false`: replay could repeat committed writes. REST problem details, operations error bodies and terminal stream records expose the flag. This is local commit, the same durability as success, not a peer or fsync guarantee. A failed commit ends its scope; it cannot resume writes. Only a transaction's root link inherits its context's count; LMDB's write-bearing child and later database links are not automatically confirmed. Pinned by `unitTests/resources/replicationConfirmationOutcome.test.js` and `unitTests/server/serverHelpers/serverErrorHandler.test.js`. A confirmation error from an independent nested transaction cannot attest to an enclosing scope's pending writes: the commit and callback failure funnels remove its flag before aborting those writes. Abort and terminal-failure cleanup latch discarded writes on the logical scope's root, so an earlier timeout, disconnect or explicit abort cannot erase that evidence. Committed or still-submitted writes and reused self-committing instances do not set the latch. The funnels keep the original error in `cause` and `retryable: false` in programmatic and HTTP errors because the nested write did land. The flag attests to the reported commit's issued writes, not later application steps that never ran after a rejection; a nested flag can remain when the enclosing scope has neither pending nor previously discarded writes. Programmatic errors are `ReplicationConfirmationError` wrappers, not the original instance; inspect `cause` for its class and identity. String-valued errors (native HTTP handlers, WebSocket close reasons and JSON-array stream elements) render `ReplicationConfirmationError`. Object responses preserve the original public code alongside the flag. Confirmation wrapping and flag removal copy diagnostics best-effort, preserving the cause and outcome when getters fail. Unreadable structured confirmation details fall back to the ordinary error response. HTTP guards cover the `replicationUnconfirmed`/`retryable` extensions and structured confirmation details. REST and operations renderers' base reads of `statusCode`, `code`, `message` and `detail` are not hardened for arbitrary application errors. diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 7188576caa..4810a3ed2d 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -1035,6 +1035,8 @@ export class DatabaseTransaction implements Transaction { declare scopeAbandoned?: boolean; declare abandonedLinks?: DatabaseTransaction[]; committingWrites = false; + // Root-scoped history survives abort cleanup until the logical scope ends. + declare writesRolledBack?: boolean; rangeReadActive = false; @@ -3186,6 +3188,12 @@ export class DatabaseTransaction implements Transaction { this.releaseContext(!!options.doneWriting); } if (!hasDurableLog(transaction, error as any)) { + if ( + !isImmediateCommit && + !this.saveCommits && + this.writes.some((write) => write && (!write.stagedOn || write.stagedOn === transaction)) + ) + (this.root ?? this).writesRolledBack = true; for (const write of this.writes) { if ( !write?.saved || @@ -3380,7 +3388,7 @@ export class DatabaseTransaction implements Transaction { protected abortAfterCommitError(error, targetTransaction?: RocksTransaction): never { try { - if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) + if (error?.replicationUnconfirmed === true && (this.hasPendingWrites() || (this.root ?? this).writesRolledBack)) error = withoutReplicationUnconfirmed(error); } catch {} try { @@ -3415,6 +3423,20 @@ export class DatabaseTransaction implements Transaction { isImmediateTarget || this.hasWriteOwnedByActiveCommit(targetTransaction, hasSiblingInFlight, true); const preserveNonImmediateActiveWrites = isImmediateTarget || this.hasWriteOwnedByActiveCommit(targetTransaction, hasSiblingInFlight, true, true); + if ( + !isImmediateTarget && + !this.saveCommits && + this.writes.some((write) => { + if ( + !write || + (preserveActiveWrites && this.isWriteOwnedByActiveCommit(write, targetTransaction, hasSiblingInFlight, true)) + ) + return false; + const owner = write.stagedOn ?? this.activeCommitHandle ?? nativeTxn; + return !this.isHandleCommitted(owner) && !hasDurableLog(owner); + }) + ) + (this.root ?? this).writesRolledBack = true; const cascadeNext = cascade && !preserveNonImmediateActiveWrites; const next = cascadeNext ? this.next : undefined; if (cascadeNext && !retainReadTransaction) this.next = null; diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index 9a4d5e51f3..acf50701f4 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -402,9 +402,10 @@ export class LMDBTransaction extends DatabaseTransaction { private abortCommitFailure(error, next: DatabaseTransaction | undefined): never { try { - if (error?.replicationUnconfirmed === true && this.hasPendingWrites()) + if (error?.replicationUnconfirmed === true && (this.hasPendingWrites() || (this.root ?? this).writesRolledBack)) error = withoutReplicationUnconfirmed(error); } catch {} + if (!this.saveCommits && this.writes.some((write) => write)) (this.root ?? this).writesRolledBack = true; const nextWasAttached = Boolean(next && this.next === next); try { this.abort(true); @@ -433,6 +434,8 @@ export class LMDBTransaction extends DatabaseTransaction { } abort(retainReadTransaction = false, cascade = true): void { + if (!this.saveCommits && !this.nativeCommitSubmitted && this.writes.some((write) => write)) + (this.root ?? this).writesRolledBack = true; const next = cascade ? this.next : undefined; if (cascade && !retainReadTransaction) this.next = null; try { diff --git a/resources/transaction.ts b/resources/transaction.ts index db9d00238a..a81fe0163e 100644 --- a/resources/transaction.ts +++ b/resources/transaction.ts @@ -109,7 +109,7 @@ export function transaction( if ( callbackThrew && error?.replicationUnconfirmed === true && - (transaction.hasPendingWrites() || transaction.isChainCommitting()) + (transaction.hasPendingWrites() || transaction.isChainCommitting() || transaction.writesRolledBack) ) error = withoutReplicationUnconfirmed(error); } catch {} diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index b454177004..94546e4638 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -400,6 +400,194 @@ describe('Replication confirmation failure after a local commit', () => { assert.equal(await Second.get(`outer-${id}`), null); }); + for (const sourceFailure of [false, true]) { + for (const abort of ['timeout', 'disconnect', 'explicit']) { + it(`removes a nested flag after ${abort} discarded outer writes in ${sourceFailure ? 'source work' : 'the callback'}`, async () => { + const id = nextId(); + const gate = Promise.withResolvers(); + const controller = new AbortController(); + const context = { signal: controller.signal }; + const failure = new ServerError('Nested peer timeout', 504); + let entered = false; + let scope; + let settled = false; + registerConfirmation(() => { + entered = true; + return gate.promise; + }); + class OuterSource extends Second {} + OuterSource.sourcedFrom({ get() {}, put: () => confirmedWrite(`inner-${id}`) }); + const completing = transaction(context, async (txn) => { + scope = txn; + await (sourceFailure ? OuterSource : Second).put({ id: `outer-${id}` }); + if (!sourceFailure) await confirmedWrite(`inner-${id}`); + }).then( + () => (settled = true), + (error) => { + settled = true; + return error; + } + ); + try { + await waitFor(() => entered); + assert.strictEqual(scope.hasPendingWrites(), true); + if (abort === 'timeout') scope.abortDueToTimeout(); + else if (abort === 'disconnect') controller.abort(); + else scope.abort(); + assert.strictEqual(scope.hasPendingWrites(), false); + if (!sourceFailure) assert.strictEqual(scope.isChainCommitting(), false); + gate.reject(failure); + await waitFor(() => settled); + const error = await completing; + assert.strictEqual(error.replicationUnconfirmed, undefined); + assert.strictEqual(error.cause.replicationUnconfirmed, true); + assert.strictEqual(error.retryable, false); + assert.strictEqual((await First.get(`inner-${id}`)).id, `inner-${id}`); + assert.equal(await Second.get(`outer-${id}`), null); + } finally { + gate.resolve(); + await waitFor(() => settled); + await completing; + } + }); + } + } + + for (const explicit of [false, true]) { + for (const poison of ['timeout', 'disconnect']) { + it(`keeps an own committed outcome after ${poison}${explicit ? ' in an explicitly awaited commit' : ''}`, async () => { + const id = nextId(); + const gate = Promise.withResolvers(); + const controller = new AbortController(); + const context = { replicatedConfirmation: 1, signal: controller.signal }; + let entered = false; + let scope; + let settled = false; + registerConfirmation(() => { + entered = true; + return gate.promise; + }); + const completing = transaction(context, async (txn) => { + scope = txn; + await First.put({ id }); + requestConfirmation(context); + if (explicit) await txn.commit(); + }).then( + () => (settled = true), + (error) => { + settled = true; + return error; + } + ); + try { + await waitFor(() => entered); + if (poison === 'timeout') scope.abortDueToTimeout(); + else controller.abort(); + gate.reject(new ServerError('Own peer timeout', 504)); + await waitFor(() => settled); + const error = await completing; + assert.strictEqual(error.replicationUnconfirmed, true); + assert.strictEqual(error.retryable, false); + assert.strictEqual((await First.get(id)).id, id); + } finally { + gate.resolve(); + await waitFor(() => settled); + await completing; + } + }); + } + } + + it('keeps the committed outcome when its synchronous confirmation hook aborts the scope', async () => { + const id = nextId(); + const context = { replicatedConfirmation: 1 }; + let scope; + registerConfirmation(() => { + scope.abort(); + throw new ServerError('Own peer timeout', 504); + }); + await assert.rejects( + transaction(context, async (txn) => { + scope = txn; + await First.put({ id }); + requestConfirmation(context); + }), + (error) => error.replicationUnconfirmed === true && error.retryable === false + ); + assert.strictEqual((await First.get(id)).id, id); + }); + + it('does not certify outer writes after a swallowed local commit failure', async () => { + const id = nextId(); + const context = {}; + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + await assert.rejects( + transaction(context, async (scope) => { + if (isLMDB) { + await Second.getResource(`outer-${id}`, context, {}); + let link = scope; + while (link.db?.path !== Second.primaryStore.path) link = link.next; + link.addWrite({ + key: `failed-${id}`, + store: Second.primaryStore, + commit() { + throw new Error('Application write failed inside the native batch'); + }, + }); + } + await Second.put({ id: `outer-${id}` }); + if (!isLMDB) { + let link = scope; + while (link.db?.path !== Second.primaryStore.path) link = link.next; + link.transaction.abort(); + } + const previousRisk = Second.primaryStore.retryRisk; + if (isLMDB) Second.primaryStore.retryRisk = 101; + try { + await assert.rejects(Promise.resolve().then(() => scope.commit())); + } finally { + Second.primaryStore.retryRisk = previousRisk; + } + assert.strictEqual(scope.hasPendingWrites(), false); + await confirmedWrite(`inner-${id}`); + }), + (error) => error.replicationUnconfirmed === undefined && error.retryable === false + ); + assert.equal(await Second.get(`outer-${id}`), null); + assert.strictEqual((await First.get(`inner-${id}`)).id, `inner-${id}`); + }); + + it('keeps a later own outcome on a reused RocksDB self-committing context after a failed write', async function () { + if (isLMDB) this.skip(); + const id = nextId(); + const context = {}; + await First.getResource(id, context, {}); + const scope = context.transaction; + assert.strictEqual(scope.saveCommits, true); + class Failing extends First {} + Failing.sourcedFrom({ + get() {}, + put() { + throw new ServerError('Source write failed', 502); + }, + }); + const failing = await Failing.getResource(`failed-${id}`, context, {}); + await assert.rejects( + Promise.resolve().then(() => failing.put({ id: `failed-${id}` }, { id: `failed-${id}` })), + /Source write failed/ + ); + assert.strictEqual(context.transaction, scope); + scope.replicatedConfirmation = 1; + registerConfirmation(() => Promise.reject(new ServerError('Own peer timeout', 504))); + const later = await First.getResource(`later-${id}`, context, {}); + await assert.rejects( + Promise.resolve().then(() => later.put({ id: `later-${id}` }, { id: `later-${id}` })), + (error) => error.replicationUnconfirmed === true && error.retryable === false + ); + assert.equal(await First.get(`failed-${id}`), null); + assert.strictEqual((await First.get(`later-${id}`)).id, `later-${id}`); + }); + it('removes a nested outcome flag even when structured error details cannot be copied', async () => { const id = nextId(); const failure = new ServerError('Nested peer timeout', 504); @@ -633,8 +821,8 @@ describe('Replication confirmation failure after a local commit', () => { }); } - for (const rollback of [false, true]) { - it(`publishes the REST outcome${rollback ? ' after outer rollback' : ''} while retaining the original public code`, async () => { + for (const rollback of [false, true, 'aborted']) { + it(`publishes the REST outcome${rollback === 'aborted' ? ' after an earlier outer abort' : rollback ? ' after outer rollback' : ''} while retaining the original public code`, async () => { const id = nextId(); registerConfirmation(() => Promise.reject(new ServerError('Peer timeout', 504))); class Signup extends Resource { @@ -642,6 +830,7 @@ describe('Replication confirmation failure after a local commit', () => { async post(target) { if (rollback) { await Second.put({ id: `outer-rest-${target.id}` }); + if (rollback === 'aborted') contextStorage.getStore().transaction.abort(); await confirmedWrite(target.id); } else { await First.put({ id: target.id }); From 70b4e8d41575083dd60c04dd5dfae67049971d90 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 20 Oct 2026 11:41:27 -0600 Subject: [PATCH 13/14] Avoid capturing rollback scans during empty transaction cleanup Co-Authored-By: OpenAI Codex GPT-6.1 --- resources/DatabaseTransaction.ts | 30 +++++++++++++------ resources/LMDBTransaction.ts | 7 ++++- .../replicationConfirmationOutcome.test.js | 15 ++++++++++ 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 4810a3ed2d..6fc9cf4d33 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -1668,6 +1668,25 @@ export class DatabaseTransaction implements Transaction { return false; } + private hasWriteToDiscard( + nativeTxn: RocksTransaction | null | undefined, + targetTransaction: RocksTransaction | undefined, + hasSiblingInFlight: boolean, + preserveActiveWrites: boolean + ): boolean { + for (let i = 0; i < this.writes.length; i++) { + const write = this.writes[i]; + if ( + !write || + (preserveActiveWrites && this.isWriteOwnedByActiveCommit(write, targetTransaction, hasSiblingInFlight, true)) + ) + continue; + const owner = write.stagedOn ?? this.activeCommitHandle ?? nativeTxn; + if (!this.isHandleCommitted(owner) && !hasDurableLog(owner)) return true; + } + return false; + } + private hasSavedWriteForHandle(transaction: RocksTransaction, isImmediateCommit: boolean): boolean { for (let i = 0; i < this.writes.length; i++) { const write = this.writes[i]; @@ -3424,17 +3443,10 @@ export class DatabaseTransaction implements Transaction { const preserveNonImmediateActiveWrites = isImmediateTarget || this.hasWriteOwnedByActiveCommit(targetTransaction, hasSiblingInFlight, true, true); if ( + this.writes.length > 0 && !isImmediateTarget && !this.saveCommits && - this.writes.some((write) => { - if ( - !write || - (preserveActiveWrites && this.isWriteOwnedByActiveCommit(write, targetTransaction, hasSiblingInFlight, true)) - ) - return false; - const owner = write.stagedOn ?? this.activeCommitHandle ?? nativeTxn; - return !this.isHandleCommitted(owner) && !hasDurableLog(owner); - }) + this.hasWriteToDiscard(nativeTxn, targetTransaction, hasSiblingInFlight, preserveActiveWrites) ) (this.root ?? this).writesRolledBack = true; const cascadeNext = cascade && !preserveNonImmediateActiveWrites; diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index acf50701f4..63983ebece 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -434,7 +434,12 @@ export class LMDBTransaction extends DatabaseTransaction { } abort(retainReadTransaction = false, cascade = true): void { - if (!this.saveCommits && !this.nativeCommitSubmitted && this.writes.some((write) => write)) + if ( + this.writes.length > 0 && + !this.saveCommits && + !this.nativeCommitSubmitted && + this.writes.some((write) => write) + ) (this.root ?? this).writesRolledBack = true; const next = cascade ? this.next : undefined; if (cascade && !retainReadTransaction) this.next = null; diff --git a/unitTests/resources/replicationConfirmationOutcome.test.js b/unitTests/resources/replicationConfirmationOutcome.test.js index 94546e4638..d863bf14a9 100644 --- a/unitTests/resources/replicationConfirmationOutcome.test.js +++ b/unitTests/resources/replicationConfirmationOutcome.test.js @@ -158,6 +158,21 @@ describe('Replication confirmation failure after a local commit', () => { ); }); + it('keeps a nested committed outcome after aborting an empty read-only scope', async () => { + const id = nextId(); + registerConfirmation(() => Promise.reject(new ServerError('Nested peer timeout', 504))); + await assert.rejects( + transaction({}, async (scope) => { + await Second.get(`absent-${id}`); + assert.strictEqual(scope.hasPendingWrites(), false); + scope.abort(); + await confirmedWrite(id); + }), + (error) => error.replicationUnconfirmed === true && error.retryable === false + ); + assert.strictEqual((await First.get(id)).id, id); + }); + for (const sourceFailure of [false, true]) { it(`rolls back when outcome metadata throws in ${sourceFailure ? 'pre-commit source work' : 'the callback'}`, async () => { const id = nextId(); From 2913e3c4de2e0ca1bd632534e29c5657fc5eeb07 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 20 Oct 2026 12:11:07 -0600 Subject: [PATCH 14/14] Describe earlier rollbacks in terminal outcome documentation Co-Authored-By: OpenAI Codex GPT-6.1 --- server/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/DESIGN.md b/server/DESIGN.md index f69bea5376..d206b7b846 100644 --- a/server/DESIGN.md +++ b/server/DESIGN.md @@ -230,7 +230,7 @@ Generic JSON-array streaming retains its `{ error: ": " }` elemen REST problem details publish only an explicitly negative `retryable: false` hint; a missing hint is unknown. All three terminal records add `replicationUnconfirmed: true` for a reported commit whose local writes landed but whose confirmation failed, and `retryable: false` for that outcome or another explicitly non-retryable error. -An enclosing rollback of pending writes removes the confirmation flag while preserving the negative retry hint. Metadata getters +An enclosing scope that rolls back, or has already discarded, writes removes the confirmation flag while preserving the negative retry hint. Metadata getters on a thrown value may fail; omit unreadable extensions and still finish a valid error record. The commit-level scope and producer coverage are described in [resources/DESIGN.md](../resources/DESIGN.md#replication-confirmation-failure-means-the-linked-writes-committed-locally). The `error` value is the stable programmatic discriminator; `message` is diagnostic and follows the same thrown-message exposure policy as pre-commit Problem Details.