diff --git a/doc/api/http.md b/doc/api/http.md index 4c1c5ad1321a..2e1bc3733e86 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -4323,6 +4323,13 @@ the following events will be emitted in the following order: `'Error: aborted'` and code `'ECONNRESET'` * `'close'` on the `res` object +If a socket error (such as a TLS error) causes the premature close, that error +is emitted on the request before the close. The error emitted on the incomplete +response retains the message `'aborted'` and code `'ECONNRESET'`, with the original +socket error available as its `cause`. This also applies when the original socket +error has code `'ECONNRESET'`. If no underlying error is available, the response +error has no `cause` property. + If `req.destroy()` is called before a socket is assigned, the following events will be emitted in the following order: @@ -4350,7 +4357,8 @@ events will be emitted in the following order: * `'aborted'` on the `res` object * `'close'` * `'error'` on the `res` object with an error with message `'Error: aborted'` - and code `'ECONNRESET'`, or the error with which `req.destroy()` was called + and code `'ECONNRESET'`. If an error was passed to `req.destroy()`, it is + available as the response error's `cause`. * `'close'` on the `res` object If `req.abort()` is called before a socket is assigned, the following diff --git a/lib/_http_client.js b/lib/_http_client.js index adcacb752e6e..20f6b0e0dc0a 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -730,7 +730,8 @@ function socketCloseListener() { if (res) { // Socket closed before we emitted 'end' below. if (!res.complete) { - res.destroy(new ConnResetException('aborted')); + res.destroy(new ConnResetException('aborted', + req[kError] ? { cause: req[kError] } : undefined)); } req._closed = true; req.emit('close'); @@ -771,6 +772,9 @@ function socketErrorListener(err) { // and we need to make sure we don't double-fire the error event. socket._hadError = true; emitErrorEvent(req, err); + // Preserve the socket error for an incomplete response, including TLS + // errors that are emitted without setting socket.errored. + req[kError] ||= err; } const parser = socket.parser; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5202737453b3..2c2a37141a38 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -845,8 +845,8 @@ class DNSException extends Error { } class ConnResetException extends Error { - constructor(msg) { - super(msg); + constructor(msg, options) { + super(msg, options); this.code = 'ECONNRESET'; } diff --git a/test/parallel/test-http-response-socket-error.js b/test/parallel/test-http-response-socket-error.js new file mode 100644 index 000000000000..1b62eb7cb063 --- /dev/null +++ b/test/parallel/test-http-response-socket-error.js @@ -0,0 +1,82 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const { Writable, pipeline } = require('stream'); + +for (const method of ['emit', 'emitAndDestroy', 'socketDestroy', 'requestDestroy', 'reset', 'close']) { + let error = method === 'close' || method === 'reset' ? null : new Error('Socket failure'); + let serverSocket; + const server = http.createServer(common.mustCall((req, res) => { + serverSocket = req.socket; + res.writeHead(200, { 'Content-Length': 100 }); + res.write('partial body'); + })); + + server.listen(0, common.mustCall(() => { + const req = http.get({ + port: server.address().port, + agent: false, + }, common.mustCall((res) => { + res.on('aborted', common.mustCall()); + res.on('close', common.mustCall()); + pipeline(res, new Writable({ + write(chunk, encoding, callback) { + callback(); + }, + }), common.mustCall((err) => { + assert.strictEqual(res.complete, false); + assert.strictEqual(res.aborted, true); + assert.strictEqual(err.code, 'ECONNRESET'); + assert.strictEqual(err.message, 'aborted'); + assert.strictEqual(res.errored, err); + if (error) { + assert.notStrictEqual(err, error); + assert.strictEqual(err.cause, error); + assert.deepStrictEqual(Object.getOwnPropertyDescriptor(err, 'cause'), { + value: error, + writable: true, + enumerable: false, + configurable: true, + }); + } else { + assert.strictEqual(Object.hasOwn(err, 'cause'), false); + } + server.close(common.mustCall()); + })); + + switch (method) { + case 'emit': + case 'emitAndDestroy': + // TLSSocket can emit an error without setting socket.errored. + req.socket.emit('error', error); + break; + case 'socketDestroy': + req.socket.destroy(error); + break; + case 'requestDestroy': + req.destroy(error); + break; + case 'reset': + serverSocket.resetAndDestroy(); + break; + case 'close': + req.socket.destroy(); + break; + } + })); + req.on('error', method !== 'close' ? common.mustCall((err) => { + if (method === 'reset') { + assert.strictEqual(err.code, 'ECONNRESET'); + assert.strictEqual(err.syscall, 'read'); + error = err; + } else { + assert.strictEqual(err, error); + } + if (method === 'emitAndDestroy') + req.destroy(); + }) : common.mustNotCall()); + req.on('close', common.mustCall()); + })); +} diff --git a/test/parallel/test-https-response-tls-error.js b/test/parallel/test-https-response-tls-error.js new file mode 100644 index 000000000000..63437282c5e2 --- /dev/null +++ b/test/parallel/test-https-response-tls-error.js @@ -0,0 +1,83 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const https = require('https'); +const { Writable, pipeline } = require('stream'); +const fixtures = require('../common/fixtures'); + +// Verify error propagation after receiving response headers. The invalid +// record deliberately causes a TLS error; it does not reproduce #66001's +// failure to decrypt an intact stream under backpressure. +for (const version of ['TLSv1.2', 'TLSv1.3']) { + let transport; + let requestError; + const events = []; + const server = https.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + minVersion: version, + maxVersion: version, + }, common.mustCall((req, res) => { + res.writeHead(200, { 'Content-Length': 100 }); + res.write('partial body'); + })); + server.on('connection', common.mustCall((socket) => { + transport = socket; + })); + + server.listen(0, common.mustCall(() => { + const req = https.get({ + port: server.address().port, + rejectUnauthorized: false, + agent: false, + }, common.mustCall((res) => { + assert.strictEqual(res.complete, false); + res.on('aborted', common.mustCall(() => events.push('aborted'))); + res.on('error', common.mustCall((err) => { + events.push('response error'); + assert.strictEqual(err.code, 'ECONNRESET'); + assert.strictEqual(err.message, 'aborted'); + assert.strictEqual(err.cause, requestError); + })); + res.on('close', common.mustCall(() => { + events.push('response close'); + assert.deepStrictEqual(events, [ + 'request error', + 'aborted', + 'request close', + 'response error', + 'response close', + ]); + })); + + pipeline(res, new Writable({ + write(chunk, encoding, callback) { + callback(); + }, + }), common.mustCall((err) => { + assert.strictEqual(err, res.errored); + assert.strictEqual(err.cause, requestError); + assert.strictEqual(res.complete, false); + assert.strictEqual(res.aborted, true); + server.close(common.mustCall()); + transport.destroy(); + })); + + // Bypass the server's TLSSocket to send an invalid application-data + // record after the client has received part of the HTTP response. + const record = Buffer.alloc(37); + record.set([23, 3, 3, 0, 32]); + transport.write(record); + })); + req.on('error', common.mustCall((err) => { + events.push('request error'); + assert.match(err.code, /^ERR_SSL_/); + requestError = err; + })); + req.on('close', common.mustCall(() => events.push('request close'))); + })); +}