flush() and compact() never settle their promise on a read-only database

[kriszyp]

Database.flush() and Database.compact() return a promise that can never settle when the database was opened read-only. Neither the resolve nor the reject callback is invoked — the native method just returns undefined.

Where

Database::Flush takes resolve/reject as argv[0]/argv[1] and returns before creating the async work:

// src/binding/database/database.cpp:579
napi_value Database::Flush(napi_env env, napi_callback_info info) {
	NAPI_METHOD_ARGV(2);
	UNWRAP_DB_HANDLE_AND_OPEN();

	if ((*dbHandle)->descriptor->readOnly) {
		NAPI_RETURN_UNDEFINED();   // <-- resolve/reject never called
	}

	napi_value resolve = argv[0];
	napi_value reject = argv[1];
	...

Database::Compact has the identical shape at src/binding/database/database.cpp:360.

The JS wrapper has no read-only guard of its own, so the caller is handed a forever-pending promise:

// src/database.ts:388
flush(): Promise<void> {
	return new Promise((resolve, reject) => this.store.db.flush(resolve, reject));
}

Why it looks intentional but isn't

The early return is correct for the sync siblings — FlushSync (database.cpp:561) and CompactSync (database.cpp:360) return undefined and a no-op is a fine answer for a read-only database. The async pair inherited the same guard, but for them "return undefined" is not a no-op: the promise contract makes it a permanent hang, and await db.flush() never resumes.

Impact

Any await store.flush() on a read-only handle wedges its caller for the life of the process, with no error, no log line, and a fully live event loop. If that await sits in a shared path (a durability gate, an apply loop, a shutdown sequence), everything behind it wedges with it.

Not currently reachable from Harper's replication receive path — the only read-only opens in harper are in integrationTests/qa-scratch/ — so this is a latent bug rather than a live incident. It surfaced while auditing unbounded native awaits behind harper-pro#678.

Suggested fix

Resolve immediately instead of returning, so async matches the sync no-op semantics:

if ((*dbHandle)->descriptor->readOnly) {
	napi_value undefined;
	NAPI_STATUS_THROWS(::napi_get_undefined(env, &undefined));
	napi_value result;
	NAPI_STATUS_THROWS(::napi_call_function(env, undefined, argv[0], 0, nullptr, &result));
	NAPI_RETURN_UNDEFINED();
}

Rejecting would also be defensible (flush() on a read-only database is arguably a caller error), but resolving keeps the existing "read-only flush is a no-op" contract that flushSync already establishes, so it can't break a caller that flushes opportunistically.

Either way the invariant worth enforcing is broader than these two methods: every binding entry point that accepts resolve/reject must settle exactly one of them on every path. A quick audit of the other napi_create_async_work call sites for the same early-return shape would be worthwhile.

Related, lower priority

The complete callback drops both callbacks when N-API reports cancellation:

// src/binding/database/database.cpp:615
if (status != napi_cancelled) {
	... callResolve / callReject ...
}
delete state;

Nothing in the binding calls napi_cancel_async_work today, so this is unreachable in practice — but it's the same "promise abandoned" shape and would be worth closing off alongside the fix.

---

Generated by Claude Opus 5.