review: align return()/throw() delegation with reviewers' suggested form

Both review bots (gemini-code-assist, claude) suggested the identical shape:
always define return()/throw() on the wrapper, delegate to the source when it
implements them, otherwise fall back to the protocol defaults (return -> done,
throw -> rethrow), marking stopped first so a later next() can't re-enter.
Adopt that verbatim so the applied fix matches the inline suggestions exactly.
Functionally equivalent to the prior conditional form for the current rocksdb-js
query iterator (which implements neither), and still forwards cleanup if a future
iterator adds it. Unit test updated for the always-defined defaults.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: heskew

resources/replayLogsGuards.ts

@@ -63,7 +63,7 @@ export function endIteratorOnCorruptFrame<T>(
 	onCorruptFrame: (error: RangeError) => void
 ): IterableIterator<T> {
 	let stopped = false;
-	const wrapped: IterableIterator<T> = {
+	return {
 		[Symbol.iterator]() {
 			return this;
 		},
@@ -84,19 +84,19 @@ export function endIteratorOnCorruptFrame<T>(
 				return { done: true, value: undefined };
 			}
 		},
-	};
-	// Stay a faithful proxy: delegate the optional return()/throw() so early termination
-	// (a for-of break, or an outer .return()) still releases whatever the source iterator
-	// holds. The current rocksdb-js query iterator implements neither, but a future one that
-	// adds cleanup must not be silently bypassed by this wrapper.
-	if (iterator.return) {
-		wrapped.return = (value?: any): IteratorResult<T> => {
+		// Forward early termination so the source iterator's cleanup (e.g. releasing a
+		// rocksdb read handle / lock) still runs when a consumer exits a for-of early via
+		// break/return/throw. Mark stopped first so a later next() can't re-enter. The
+		// current rocksdb-js query iterator implements neither, hence the protocol defaults.
+		return(value?: any): IteratorResult<T> {
+			stopped = true;
+			if (typeof iterator.return === 'function') return iterator.return(value);
+			return { done: true, value };
+		},
+		throw(error?: any): IteratorResult<T> {
 			stopped = true;
-			return iterator.return!(value);
-		};
-	}
-	if (iterator.throw) {
-		wrapped.throw = (error?: any): IteratorResult<T> => iterator.throw!(error);
-	}
-	return wrapped;
+			if (typeof iterator.throw === 'function') return iterator.throw(error);
+			throw error;
+		},
+	};
 }

unitTests/resources/replayLogs.test.js

@@ -147,16 +147,26 @@ describe('endIteratorOnCorruptFrame', () => {
 		assert.strictEqual(threwWith, boom);
 	});
 
-	it('does not synthesize return()/throw() when the underlying iterator lacks them', () => {
-		const wrapped = endIteratorOnCorruptFrame(
-			{
-				next() {
-					return { done: true, value: undefined };
-				},
+	it('return()/throw() fall back to protocol defaults and latch when the underlying lacks them', () => {
+		let nextCalls = 0;
+		const source = {
+			next() {
+				nextCalls++;
+				return { done: false, value: 1 };
 			},
-			() => {}
+		};
+		const wrapped = endIteratorOnCorruptFrame(source, () => {});
+
+		// return() defaults to done and latches without ever pulling the source again
+		assert.deepStrictEqual(wrapped.return('x'), { done: true, value: 'x' });
+		assert.deepStrictEqual(wrapped.next(), { done: true, value: undefined });
+		assert.strictEqual(nextCalls, 0);
+
+		// throw() rethrows when the source can't handle it
+		const boom = new Error('boom');
+		assert.throws(
+			() => endIteratorOnCorruptFrame({ next: source.next }, () => {}).throw(boom),
+			(error) => error === boom
 		);
-		assert.strictEqual(wrapped.return, undefined);
-		assert.strictEqual(wrapped.throw, undefined);
 	});
 });