Skip to content

Commit

Permalink
http: make OutgoingMessage more streamlike
Browse files Browse the repository at this point in the history
Implement missing getters error & closed. Add support for
proper "writable" check through `isWritable` helper.

We cannot fix the `OutgoingMessage.writable` property as that
would break the ecosystem.

PR-URL: #45672
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
ronag authored and danielleadams committed Jan 3, 2023
1 parent 55f800b commit cbd710b
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 21 deletions.
19 changes: 19 additions & 0 deletions lib/_http_outgoing.js
Expand Up @@ -88,6 +88,7 @@ const kCorked = Symbol('corked');
const kUniqueHeaders = Symbol('kUniqueHeaders');
const kBytesWritten = Symbol('kBytesWritten');
const kEndCalled = Symbol('kEndCalled');
const kErrored = Symbol('errored');

const nop = () => {};

Expand Down Expand Up @@ -147,10 +148,26 @@ function OutgoingMessage() {
this._keepAliveTimeout = 0;

this._onPendingData = nop;

this[kErrored] = null;
}
ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype);
ObjectSetPrototypeOf(OutgoingMessage, Stream);

ObjectDefineProperty(OutgoingMessage.prototype, 'errored', {
__proto__: null,
get() {
return this[kErrored];
},
});

ObjectDefineProperty(OutgoingMessage.prototype, 'closed', {
__proto__: null,
get() {
return this._closed;
},
});

ObjectDefineProperty(OutgoingMessage.prototype, 'writableFinished', {
__proto__: null,
get() {
Expand Down Expand Up @@ -320,6 +337,8 @@ OutgoingMessage.prototype.destroy = function destroy(error) {
}
this.destroyed = true;

this[kErrored] = error;

if (this.socket) {
this.socket.destroy(error);
} else {
Expand Down
9 changes: 8 additions & 1 deletion test/parallel/test-http-client-incomingmessage-destroy.js
Expand Up @@ -20,6 +20,13 @@ server.listen(0, () => {
get({
port: server.address().port
}, common.mustCall((res) => {
res.destroy(new Error('Destroy test'));
const err = new Error('Destroy test');
assert.strictEqual(res.errored, null);
res.destroy(err);
assert.strictEqual(res.closed, false);
assert.strictEqual(res.errored, err);
res.on('close', () => {
assert.strictEqual(res.closed, true);
});
}));
});
71 changes: 51 additions & 20 deletions test/parallel/test-http-outgoing-destroyed.js
@@ -1,24 +1,55 @@
'use strict';
const common = require('../common');
const http = require('http');
const assert = require('assert');

const server = http.createServer(common.mustCall((req, res) => {
req.pipe(res);
res.on('error', common.mustNotCall());
res.on('close', common.mustCall(() => {
res.end('asd');
process.nextTick(() => {
server.close();
});
}));
})).listen(0, () => {
http
.request({
port: server.address().port,
method: 'PUT'
})
.on('response', (res) => {
res.destroy();
})
.write('asd');
});
{
const server = http.createServer(common.mustCall((req, res) => {
assert.strictEqual(res.closed, false);
req.pipe(res);
res.on('error', common.mustNotCall());
res.on('close', common.mustCall(() => {
assert.strictEqual(res.closed, true);
res.end('asd');
process.nextTick(() => {
server.close();
});
}));
})).listen(0, () => {
http
.request({
port: server.address().port,
method: 'PUT'
})
.on('response', (res) => {
res.destroy();
})
.write('asd');
});
}

{
const server = http.createServer(common.mustCall((req, res) => {
assert.strictEqual(res.closed, false);
req.pipe(res);
res.on('error', common.mustNotCall());
res.on('close', common.mustCall(() => {
assert.strictEqual(res.closed, true);
process.nextTick(() => {
server.close();
});
}));
const err = new Error('Destroy test');
res.destroy(err);
assert.strictEqual(res.errored, err);
})).listen(0, () => {
http
.request({
port: server.address().port,
method: 'PUT'
})
.on('error', common.mustCall())
.write('asd');
});

}

0 comments on commit cbd710b

Please sign in to comment.