Skip to content

Commit

Permalink
http: align OutgoingMessage.writable with streams
Browse files Browse the repository at this point in the history
Should be a computed value that looks at finished and
destroyed state.

Fixes: nodejs#33643
  • Loading branch information
ronag committed Jun 14, 2020
1 parent e1ad548 commit 7cbc07f
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 1 deletion.
14 changes: 13 additions & 1 deletion lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
const { CRLF, debug } = common;

const kCorked = Symbol('corked');
const kWritable = Symbol('writable');

function nop() {}

Expand Down Expand Up @@ -97,7 +98,6 @@ function OutgoingMessage() {
// TCP socket and HTTP Parser and thus handle the backpressure.
this.outputSize = 0;

this.writable = true;
this.destroyed = false;

this._last = false;
Expand Down Expand Up @@ -127,6 +127,18 @@ function OutgoingMessage() {
ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype);
ObjectSetPrototypeOf(OutgoingMessage, Stream);

ObjectDefineProperty(OutgoingMessage.prototype, 'writable', {
get() {
// TODO (ronag): Add errored state? write/end might error/destroy one
// tick later.
return this[kWritable] !== false && !this.destroyed && !this.finished;
},
set(val) {
// Backwards compatible.
this[kWritable] = !!val;
}
});

ObjectDefineProperty(OutgoingMessage.prototype, 'writableFinished', {
get() {
return (
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-http-outgoing-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ const OutgoingMessage = http.OutgoingMessage;
{
const msg = new OutgoingMessage();
assert.strictEqual(msg.destroyed, false);
assert.strictEqual(msg.writable, true);
msg.destroy();
assert.strictEqual(msg.destroyed, true);
assert.strictEqual(msg.writable, false);
let callbackCalled = false;
msg.write('asd', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-http-outgoing-finish.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,5 @@ function write(out) {
console.log(`ok - ${name} endCb`);
});
}));
assert.strictEqual(out.writable, false);
}
3 changes: 3 additions & 0 deletions test/parallel/test-http-outgoing-writableFinished.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ server.on('listening', common.mustCall(function() {
path: '/'
});

assert.strictEqual(clientRequest.writable, true);
assert.strictEqual(clientRequest.writableFinished, false);
clientRequest
.on('finish', common.mustCall(() => {
assert.strictEqual(clientRequest.writable, false);
assert.strictEqual(clientRequest.writableFinished, true);
}))
.end();
assert.strictEqual(clientRequest.writable, false);
assert.strictEqual(clientRequest.writableFinished, false);
}));

0 comments on commit 7cbc07f

Please sign in to comment.