address review: delegate return()/throw() in endIteratorOnCorruptFrame

Both PR review bots flagged that the corrupt-frame iterator wrapper dropped the
underlying iterator's optional return()/throw(), so early termination (a for-of
break or an outer .return()) would skip the source iterator's cleanup. The
current rocksdb-js query iterator implements neither, so nothing leaks today, but
the wrapper should stay a faithful proxy — delegate both when present (guarded by
typeof), and latch stopped on return(). Adds unit tests for delegation and for
the no-synthesis-when-absent case.

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;
-	return {
+	const wrapped: IterableIterator<T> = {
 		[Symbol.iterator]() {
 			return this;
 		},
@@ -85,4 +85,18 @@ export function endIteratorOnCorruptFrame<T>(
 			}
 		},
 	};
+	// 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> => {
+			stopped = true;
+			return iterator.return!(value);
+		};
+	}
+	if (iterator.throw) {
+		wrapped.throw = (error?: any): IteratorResult<T> => iterator.throw!(error);
+	}
+	return wrapped;
 }

unitTests/resources/replayLogs.test.js

@@ -116,4 +116,47 @@ describe('endIteratorOnCorruptFrame', () => {
 		assert.deepStrictEqual([...wrapped], [1]);
 		assert.strictEqual(reported, 0);
 	});
+
+	it('delegates return()/throw() to the underlying iterator so early-exit cleanup runs', () => {
+		let returnedWith;
+		let threwWith;
+		const source = {
+			next() {
+				return { done: false, value: 1 };
+			},
+			return(value) {
+				returnedWith = value;
+				return { done: true, value };
+			},
+			throw(error) {
+				threwWith = error;
+				return { done: true, value: undefined };
+			},
+		};
+		const wrapped = endIteratorOnCorruptFrame(source, () => {});
+
+		assert.strictEqual(typeof wrapped.return, 'function');
+		assert.deepStrictEqual(wrapped.return('cleanup'), { done: true, value: 'cleanup' });
+		assert.strictEqual(returnedWith, 'cleanup');
+		// after return(), the wrapper is latched done and never touches the source again
+		assert.deepStrictEqual(wrapped.next(), { done: true, value: undefined });
+
+		assert.strictEqual(typeof wrapped.throw, 'function');
+		const boom = new Error('boom');
+		wrapped.throw(boom);
+		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 };
+				},
+			},
+			() => {}
+		);
+		assert.strictEqual(wrapped.return, undefined);
+		assert.strictEqual(wrapped.throw, undefined);
+	});
 });