[dhensby]

What's wrong

Request.pipe() crashes the process on any request error: a SQL error, a timeout or a cancel. It happens with both drivers.

pipe() puts an internal Readable in front of the destination and returns the destination, so callers can't listen on the Readable. toReadableStream() re-emitted each request error on that Readable, and Readable.prototype.pipe() doesn't forward a source's errors to the destination. So the 'error' had no listener and was thrown from inside the driver's event handler. Error listeners on the destination (as the README example shows) or on the request didn't help:

req.pipe(dest)   // dest.on('error', ...) attached
req.query('select * from notexistingtable')
// => uncaught RequestError: Invalid object name 'notexistingtable'.

The original change (616c745, "Enable pipe to support back pressure and errors") meant to forward errors to the stream, but the code never did.

What changes

fix: resume a request when its readable stream is destroyed

Fixing the crash depends on this. toReadableStream() pauses the request when the stream's buffer is full and resumes it only when the stream reads again. A destroyed stream never reads again, so the request stayed paused forever and kept its pooled connection. This already happened on master with pipeline(request.toReadableStream(), dest) when dest failed part way through. I reproduced it on tedious: the request hangs and the pool can't close. Now, when the stream is destroyed before the request is done, the request is resumed and no longer paused. It runs to the end, its remaining rows are dropped and its connection is released.

• A destroy hook passed in toReadableStream()'s stream options still runs, after the stream's own, with the stream as this. null options work as before.
• The request is resumed as soon as its stream is destroyed. So on msnodesqlv8 before 5.6, which can only cancel a paused query, a cancel() made after that doesn't stop it, and it runs to the end. The README says so, in line with its existing note that those versions can't cancel a running request. A cancel made while the request is still paused works as before.

fix: pass request errors to the destination of Request.pipe()

• pipe() destroys the destination with the request's first error, as pipeline() does. The destination's 'error' listener gets the error and 'finish' isn't emitted.
• toReadableStream() destroys its stream with the first error instead of calling emit('error') by hand, which follows the stream contract. pipeline() users see no difference.
• A stream request can emit several errors and still go on to 'done'. A second error is a no-op on a destroyed stream, so it stays on the request. Rows after the error and the final push(null) are dropped. Neither can throw or pause the request again.
• A cancelled request is left to its driver: when the request's error is ECANCEL, the destroyed stream does not resume it. tedious resumes a cancelled request itself. Since fix: support the msnodesqlv8 5.6 error, cancel and info events #1917, msnodesqlv8 reports a cancelled stream once the query is gone and its connection is back in the pool. Resuming it at that point made msnodesqlv8 read the freed statement, which raised cannot call dead statement / Statement not found and sent extra done/free events to a connection already back in the pool. I confirmed both the stray events and the fix on Windows CI for msnodesqlv8 2.7–5.6.
• I chose destroy(err) over writableStream.emit('error', err). Emitting by hand bypasses the Writable's state machine: the stream isn't marked errored, it still accepts writes, and it can still finish.
• README: describes the error behaviour of pipe() and toReadableStream(). For chains of streams it recommends stream.pipeline() with toReadableStream(), since pipe() only passes errors to the stream it is given.

fix: release a piped request when its destination fails

This one also happens on master. When the destination fails or closes first, Readable.prototype.pipe() only unpipes from it. Nothing destroys the internal stream, so the request stays paused and keeps its connection for good. Piping into a destination that is already destroyed does the same. I reproduced both on tedious.

• pipe() now destroys its internal stream when the destination unpipes before the stream ends, or is already destroyed. The request then runs to the end, its remaining rows are dropped and its connection is released.
• It listens for 'unpipe'. A destination created with autoDestroy or emitClose turned off never emits 'close'. And stream.finished() adds an 'error' listener, which would stop an unhandled destination error from being thrown.
• A unit test checks that pipe() adds no 'error' listener of its own.

msnodesqlv8

On master, a msnodesqlv8 stream-mode request does not complete after an error. That has nothing to do with pipe(): the same happens with request.stream = true and no pipe. I confirmed it on Windows CI with SQL Server 2022, tracing the raw driver events on every msnodesqlv8 line CI uses (2.7.0, 3.1.0, 4.5.0, 5.2.1, 5.6.0):

• Before 5.6, msnodesqlv8 stops the query at its first error, whatever the error: it emits error(more=false) then free, and never done. The adapter only completes a stream on done, so the request keeps its connection.
• From 5.6, an error that does not end the batch arrives with more=true and the batch goes on to done. An error that ends the batch still gets no done.
• With this PR, pipe() no longer crashes on any msnodesqlv8 version. The destination gets the first error on all of them. Whether the request then completes depends only on the behaviour above. pipe() and plain stream mode gave the same result in every scenario.
• Cancelling a back-pressured (paused) piped request completes on every version. msnodesqlv8 emits no error for it, only done, through the adapter's isPaused() workaround.

So the msnodesqlv8 suite:

• skips query with pipe and an error that ends the batch on every version;
• runs query with pipe and an error that does not end the batch from 5.6 only, and skips it with a reason before that.

Both can run on every version once msnodesqlv8 streams complete after an error (settling on free as well as done). The pipe logic lives in lib/base/request.js and doesn't depend on the driver; the new unit tests cover it without a database.

Testing

• New integration tests, wired for both drivers (except where msnodesqlv8 skips them, as above):
  • query with pipe and an error that ends the batch
  • query with pipe and an error that does not end the batch
  • query with toReadableStream and a failing destination
  • query with pipe and a failing destination (it checks the request was held back by the full stream when the destination failed)
  • query with pipe into a destination that is already destroyed
• Each one fails on master. The pipe error tests fail with Uncaught RequestError. The others time out with the connection still borrowed.
• New unit tests under Request readable stream. Each was mutation-tested against the change it covers. They pass on Node 18, 20, 22 and 24.
• npm run lint and npm run test-unit pass (171).
• npm run test-tedious locally against Azure SQL Edge: 169 passing. The 3 failures (user defined types and two CLR-type bulk loads) are because SQL Edge has no CLR. They fail the same way on master.
• After an autosquash, each of the three commits passes lint, unit tests and the tedious stream tests on its own.
• I also checked these by hand on tedious: two errors in one batch, an error part way through rows, a request timeout, and a cancel while the stream was back-pressured (paused). In every case the destination got the first error only, the request reached 'done', and the connection went back to the pool.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new destroy implementation discards caller-provided stream cleanup hooks.

1 open finding

• Preserve caller-supplied streamOptions.destroy hook · New

What changed in this PR

Fixes request errors crashing Request.pipe() and ensures destroyed readable streams release back-pressured requests.

Changes:

• Destroys streams with the first request error and forwards pipe errors.
• Resumes unfinished, non-cancelled requests when their readable stream is destroyed.
• Adds driver tests and documents stream error behavior.

File	Description
lib/​base/​request.js	Implements stream destruction, recovery, and error forwarding.
README.md	Documents error and pipeline behavior.
test/​common/​unit.js	Tests readable destruction and error handling.
test/​common/​tests.js	Adds shared integration scenarios.
test/​tedious/​tedious.js	Enables new tedious tests.
test/​msnodesqlv8/​msnodesqlv8.js	Adds version-gated msnodesqlv8 coverage.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

A silently destroyed destination can still leave the request paused and retain its pooled connection.

1 open finding

• Destroyed destination without close leaves source piped and connection retained · New

1 resolved since last review

• Preserve caller-supplied streamOptions.destroy hook

🧠 Review effort: Balanced

[github-actions]

🎉 This PR is included in version 12.7.7 🎉

The release is available on:

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

Your semantic-release bot 📦🚀