Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "http: align with stream.Writable" #37963

Closed
wants to merge 1 commit into from
Closed

Revert "http: align with stream.Writable" #37963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 20 additions & 32 deletions lib/_http_client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const Agent = require('_http_agent');
const { Buffer } = require('buffer');
const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
const { kOutHeaders } = require('internal/http');
const { kOutHeaders, kNeedDrain } = require('internal/http');
const { connResetException, codes } = require('internal/errors');
const {
ERR_HTTP_HEADERS_SENT,
Expand Down Expand Up @@ -98,7 +98,7 @@ class HTTPClientAsyncResource {
}

function ClientRequest(input, options, cb) {
FunctionPrototypeCall(OutgoingMessage, this, { autoDestroy: false });
FunctionPrototypeCall(OutgoingMessage, this);

if (typeof input === 'string') {
const urlStr = input;
Expand Down Expand Up @@ -304,7 +304,7 @@ function ClientRequest(input, options, cb) {
if (typeof optsWithoutSignal.createConnection === 'function') {
const oncreate = once((err, socket) => {
if (err) {
process.nextTick(() => emitError(this, err));
process.nextTick(() => this.emit('error', err));
} else {
this.onSocket(socket);
}
Expand Down Expand Up @@ -373,8 +373,8 @@ function emitAbortNT(req) {

function ondrain() {
const msg = this._httpMessage;
if (msg && !msg.finished && msg._writableState.needDrain) {
msg._writableState.needDrain = false;
if (msg && !msg.finished && msg[kNeedDrain]) {
msg[kNeedDrain] = false;
msg.emit('drain');
}
}
Expand All @@ -400,7 +400,8 @@ function socketCloseListener() {
if (!res.complete) {
res.destroy(connResetException('aborted'));
}
emitClose(req);
req._closed = true;
req.emit('close');
if (!res.aborted && res.readable) {
res.push(null);
}
Expand All @@ -410,9 +411,10 @@ function socketCloseListener() {
// receive a response. The error needs to
// fire on the request.
req.socket._hadError = true;
emitError(req, connResetException('socket hang up'));
req.emit('error', connResetException('socket hang up'));
}
emitClose(req);
req._closed = true;
req.emit('close');
}

// Too bad. That output wasn't getting written.
Expand All @@ -436,7 +438,7 @@ function socketErrorListener(err) {
// For Safety. Some additional errors might fire later on
// and we need to make sure we don't double-fire the error event.
req.socket._hadError = true;
emitError(req, err);
req.emit('error', err);
}

const parser = socket.parser;
Expand All @@ -460,7 +462,7 @@ function socketOnEnd() {
// If we don't have a response then we know that the socket
// ended prematurely and we need to emit an error on the request.
req.socket._hadError = true;
emitError(req, connResetException('socket hang up'));
req.emit('error', connResetException('socket hang up'));
}
if (parser) {
parser.finish();
Expand All @@ -483,7 +485,7 @@ function socketOnData(d) {
freeParser(parser, req, socket);
socket.destroy();
req.socket._hadError = true;
emitError(req, ret);
req.emit('error', ret);
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade (if status code 101) or CONNECT
const bytesParsed = ret;
Expand Down Expand Up @@ -515,7 +517,9 @@ function socketOnData(d) {
socket.readableFlowing = null;

req.emit(eventName, res, socket, bodyHead);
emitClose(req);
req.destroyed = true;
req._closed = true;
req.emit('close');
} else {
// Requested Upgrade or used CONNECT method, but have no handler.
socket.destroy();
Expand Down Expand Up @@ -700,7 +704,8 @@ function requestOnPrefinish() {
}

function emitFreeNT(req) {
emitClose(req);
req._closed = true;
req.emit('close');
if (req.socket) {
req.socket.emit('free');
}
Expand Down Expand Up @@ -781,10 +786,10 @@ function onSocketNT(req, socket, err) {
err = connResetException('socket hang up');
}
if (err) {
emitError(req, err);
req.emit('error', err);
}
req._closed = true;
emitClose(req);
req.emit('close');
}

if (socket) {
Expand Down Expand Up @@ -864,23 +869,6 @@ function setSocketTimeout(sock, msecs) {
}
}

function emitError(req, err) {
req.destroyed = true;
req._writableState.errored = err;
// Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364
err.stack; // eslint-disable-line no-unused-expressions
req._writableState.errorEmitted = true;
req.emit('error', err);
}

function emitClose(req) {
req.destroyed = true;
req._closed = true;
req._writableState.closed = true;
req._writableState.closeEmitted = true;
req.emit('close');
}

ClientRequest.prototype.setNoDelay = function setNoDelay(noDelay) {
this._deferToConnect('setNoDelay', [noDelay]);
};
Expand Down
Loading