[arthurschreiber]

connection.prepare() adds a returnValue listener to the request to pick up the handle that sp_prepare returns, and never removes it (see the TODO it carried). This causes two problems:

• Output parameters on prepared statements don't work. The listener stays attached for every later execute(). It treats each output parameter as an unexpected output parameter of sp_prepare, so the execution fails with Tedious > Unexpected output parameter out from sp_prepare.
• Listeners pile up. Each prepare() of the same request adds another listener.

Changes

• RequestTokenHandler.onReturnValue now stores the handle itself while the request is preparing, and reports unexpected output parameters of sp_prepare the same way as before.
• connection.prepare() no longer adds a listener.
• The returnValue event for handle is still emitted, so existing listeners keep working.

Tests

New integration tests in test/integration/prepare-execute-statements-test.ts:

• returns output parameters of executions fails on master with Unexpected output parameter out from sp_prepare.
• does not add returnValue listeners when preparing a request fails on master with expected 3 to equal +0.

Checks

• npm run lint (eslint and tsc)
• npm run build
• Unit tests: 533 passing
• Integration tests: 466 passing, 1 failing. The failure is Initiate Connect Test, which also fails on master in the local test environment.
• commitlint was not run: it isn't installed in the dev dependencies. The commit message follows the conventional format.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HGJsu1u44CHnt4nkkQ5ias

---

Generated by Claude Code

[claude]

Review

Nice, focused fix — the root cause (the returnValue listener added in connection.prepare() was never removed, and later intercepted every output parameter of execute() as an "unexpected" sp_prepare output) is correctly diagnosed, and the fix addresses it at the source rather than papering over it with listener cleanup.

Code quality

• Moving the handle-capture logic into RequestTokenHandler.onReturnValue (src/token/handler.ts) instead of a per-request listener is the right call — it eliminates the whole listener-leak class of bug rather than just this one instance.
• The this.request instanceof Request guard is correct and necessary since RequestTokenHandler.request is typed Request | BulkLoad, and BulkLoad doesn't extend Request, so this safely narrows without affecting bulk load token handling.
• Error semantics for the "unexpected output parameter" case are preserved exactly as before (request.error = new RequestError(...), consumed later at request.callback(request.error, ...)), and the returnValue event is still emitted afterward so existing consumers of that event are unaffected.
• token.value as number is an unchecked cast, but it matches the previous implicit any assignment (request.handle = value) — not a regression, just pre-existing looseness.

Correctness

• Timing checks out: request.preparing only flips back to false inside Request.callback (src/request.ts) once the whole sp_prepare RPC completes, so during onReturnValue for the prepare call preparing is still true, and by the time a later execute() runs it's false — so output parameters flow through untouched. This is the crux of the bug and it's handled correctly.
• Repeated prepare()/unprepare() cycles on the same request no longer accumulate listeners, since none are added — confirmed by the new listenerCount('returnValue') test.

Test coverage

• Both new integration tests directly reproduce the two problems described in the PR body (output params failing, and listener accumulation), and per the PR description both fail on master — good regression coverage.
• No unit test was added for the onReturnValue branch itself, but that matches the existing convention in this repo (there don't appear to be any unit tests for RequestTokenHandler or connection.prepare() currently — coverage here is integration-test only), so this isn't a gap introduced by this PR.

Security / performance: no concerns — this is a straightforward memory-leak/correctness fix with no new attack surface, and it removes an unbounded per-prepare() listener accumulation.

Overall this looks correct and well-tested. No blocking issues from my side.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.49%. Comparing base (2172243) to head (e6478d3).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/token/handler.ts	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1789      +/-   ##
==========================================
+ Coverage   81.48%   82.49%   +1.00%     
==========================================
  Files          95       95              
  Lines        4862     4862              
  Branches      913      914       +1     
==========================================
+ Hits         3962     4011      +49     
+ Misses        631      575      -56     
- Partials      269      276       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🎉 This PR is included in version 20.3.1 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀