src/core/handlers/RequestHandler.ts (1)

174-193: Consider public (or a clearly marked internal method) instead of protected for lifecycle hooks invoked externally.

reset() and restore() are declared protected, yet HandlersController (a sibling class, not a subclass) invokes them via bracket notation (handler['reset'](), handler['restore']()) specifically to bypass TypeScript's access checks. This works but couples two files with an implicit escape hatch that will silently drift if the methods are ever renamed or the signatures change — the in narrowing + index-access pattern won't produce a type error.

Either make these public (they are part of the handler lifecycle contract with the controller), or expose them through an internal symbol-keyed method so the intent is explicit. protected + bracket access is the least discoverable of the three options.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/handlers/RequestHandler.ts` around lines 174 - 193, RequestHandler's
lifecycle hooks reset() and restore() are marked protected but are invoked by
HandlersController via bracket-access; make their contract explicit by changing
them to public (i.e., declare reset() and restore() as public methods on the
RequestHandler class) so HandlersController can call handler.reset() and
handler.restore() without using index-access, or alternatively define and export
a shared internal Symbol (e.g., HANDLER_RESET/ HANDLER_RESTORE) and implement
symbol-keyed methods on RequestHandler that HandlersController uses; update all
calls in HandlersController that currently use handler['reset']() /
handler['restore']() to the chosen public method names or symbol-keyed access to
remove the protected index-access escape hatch.

test/node/rest-api/response/generator.test.ts (1)

105-122: Consider broadening test coverage to lock in the intended semantics.

The new test covers the plain generator + resetHandlers() path, which is great. To prevent regressions in the adjacent paths that this PR also touches, consider adding two more cases:

1. restoreHandlers() resets generator state for a { once: true } generator handler (the primary use case called out in the PR description).
2. A { once: true } (non-generator) handler is re-armed by resetHandlers() — or, if that's explicitly not intended, a negative test asserting it stays inert. Today it stays inert because reset() doesn't clear isUsed (see the comment on handlers-controller.ts), and this deserves an explicit contract either way.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/node/rest-api/response/generator.test.ts` around lines 105 - 122, Add
two tests to broaden coverage: (1) ensure restoreHandlers() resets the state of
a generator handler created with { once: true } by registering a generator
http.get(..., { once: true }) handler, invoking it once, calling
restoreHandlers(), then asserting the generator yields the same sequence again;
(2) assert the intended behavior for non-generator handlers with { once: true }
after resetHandlers(): either add a positive test that resetHandlers() re-arms a
non-generator { once: true } handler (invoke, resetHandlers(), expect it to fire
again) or add a negative test that it remains inert (invoke, resetHandlers(),
expect no further responses), referencing restoreHandlers(), resetHandlers(),
and the handlers-controller.ts isUsed semantics when implementing the
assertions.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/node/rest-api/response/generator.test.ts (1)

160-167: Cover regular-handler restore while the generator is still mid-flight.

This test currently calls restoreHandlers() only after the generator has reached the stable result. Add a restore between Yield and Stable to prove regular generators are not reset while active.

Proposed coverage tweak

   await server.boundary(async () => {
     await expect(fetchJson('http://localhost/resource')).resolves.toBe('Yield')
-    await expect(fetchJson('http://localhost/resource')).resolves.toBe('Stable')
 
     server.restoreHandlers()
 
+    await expect(fetchJson('http://localhost/resource')).resolves.toBe('Stable')
+
+    server.restoreHandlers()
+
     await expect(fetchJson('http://localhost/resource')).resolves.toBe('Stable')
     await expect(fetchJson('http://localhost/resource')).resolves.toBe('Stable')
   })()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/node/rest-api/response/generator.test.ts` around lines 160 - 167, Add a
call to server.restoreHandlers() between the first fetchJson assertion that
resolves to 'Yield' and the subsequent fetchJson that resolves to 'Stable' to
verify that regular generator handlers are not reset while the generator is
mid-flight; update the test inside the server.boundary block (which contains
fetchJson('http://localhost/resource') and server.restoreHandlers()) so that
server.restoreHandlers() is invoked immediately after the
expect(...).resolves.toBe('Yield') assertion and before the
expect(...).resolves.toBe('Stable') assertion, keeping the remaining assertions
the same.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/core/handlers/RequestHandler.ts (1)

180-187: ⚠️ Potential issue | 🟠 Major

Handle cleanup failures from iterator.return().

Line 186 still lets synchronous generator cleanup throw out of resetHandlers(), and async-generator cleanup rejections become unhandled because the promise is discarded. Wrap the return() call and attach a rejection handler so reset/restore cleanup remains safe.

Proposed defensive cleanup handling

   protected reset(): void {
     const iterator = this.resolverIterator
     this.resolverIterator = undefined
     this.resolverIteratorResult = undefined
 
     if (typeof iterator?.return === 'function') {
-      void Promise.resolve(iterator.return())
+      try {
+        void Promise.resolve(iterator.return()).catch(() => undefined)
+      } catch {
+        // Ignore cleanup errors during reset.
+      }
     }
   }

Verification script:

#!/bin/bash
# Demonstrates why Promise.resolve(iterator.return()) does not make cleanup safe.

node - <<'NODE'
process.on('unhandledRejection', (reason) => {
  console.log('async cleanup rejection was unhandled:', reason.message)
})

function* syncCleanupThrows() {
  try {
    yield 'value'
  } finally {
    throw new Error('sync cleanup')
  }
}

const syncIterator = syncCleanupThrows()
syncIterator.next()

try {
  void Promise.resolve(syncIterator.return())
} catch (error) {
  console.log('sync cleanup threw synchronously:', error.message)
}

async function* asyncCleanupRejects() {
  try {
    yield 'value'
  } finally {
    throw new Error('async cleanup')
  }
}

void (async () => {
  const asyncIterator = asyncCleanupRejects()
  await asyncIterator.next()

  void Promise.resolve(asyncIterator.return())

  await new Promise((resolve) => setImmediate(resolve))
})()
NODE

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/handlers/RequestHandler.ts` around lines 180 - 187, In reset(),
guard the call to this.resolverIterator.return() so synchronous throws are
caught and async rejections are handled: wrap the call in a try/catch to swallow
or log synchronous exceptions from iterator.return(), and assign
Promise.resolve(iterator.return()).catch(...) to handle any async rejection (or
await it if appropriate); update the protected reset() method (the
resolverIterator/return invocation) to use this defensive pattern so
reset/restore cleanup cannot throw or produce unhandled rejections.