Skip to content
Closed
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
40 changes: 40 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,37 @@ Emitted when the server sends a '100 Continue' HTTP response, usually because
the request contained 'Expect: 100-continue'. This is an instruction that
the client should send the request body.

### Event: 'information'
<!-- YAML
added: REPLACEME
-->

Emitted when the server sends a 1xx response (excluding 101 Upgrade). This
event is emitted with a callback containing an object with a status code.

```js
const http = require('http');

const options = {
hostname: '127.0.0.1',
port: 8080,
path: '/length_request'
};

// make a request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking nit: Using capital letters as first character would be great :-) And maybe also a dot at the end.

const req = http.request(options);
req.end();

req.on('information', (res) => {
console.log('got information prior to main response: ' + res.statusCode);
});
```

101 Upgrade statuses do not fire this event due to their break from the
traditional HTTP request/response chain, such as web sockets, in-place TLS
upgrades, or HTTP 2.0. To be notified of 101 Upgrade notices, listen for the
[`'upgrade'`]() event instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should also be added to the http2 compat API

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also explain why 101 Upgrade is not included in this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good spot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also an example illustrating what "This event is emitted with a object" means would be helpful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: Should 101s actually fire the event too?

I excluded 101 Upgrade since this was typically used to make TLS upgrades (rare) or Web Socket connections (typical). I don't work on the Socket.IO project—for example—so I don't know the potential drawbacks. I suppose performance wouldn't suffer measurably just from firing an event with no listeners.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take that back. 101 Upgrade messages are handled so differently, they can break the request/response model dramatically. Web Sockets being a prime example but TLS as well. Leaving as is.

### Event: 'response'
<!-- YAML
added: v0.1.0
Expand Down Expand Up @@ -1382,6 +1413,14 @@ which has been transmitted are equal or not.
Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

### response.writeProcessing()
<!-- YAML
added: REPLACEME
-->

Sends a HTTP/1.1 102 Processing message to the client, indicating that
the request body should be sent.

## Class: http.IncomingMessage
<!-- YAML
added: v0.1.17
Expand Down Expand Up @@ -1881,6 +1920,7 @@ const req = http.request(options, (res) => {
[`'checkContinue'`]: #http_event_checkcontinue
[`'request'`]: #http_event_request
[`'response'`]: #http_event_response
[`'upgrade'`]: #http_event_upgrade
[`Agent`]: #http_class_http_agent
[`Duplex`]: stream.html#stream_class_stream_duplex
[`EventEmitter`]: events.html#events_class_eventemitter
Expand Down
38 changes: 31 additions & 7 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,16 +447,26 @@ function socketOnData(d) {
socket.destroy();
}
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is 100 (Continue), the server will
// send a final response after this client sends a request
// body. So, we must not free the parser.
parser.incoming.statusCode !== 100) {
// When the status code is informational (100, 102-199),
// the server will send a final response after this client
// sends a request body, so we must not free the parser.
// 101 (Switching Protocols) and all other status codes
// should be processed normally.
!statusIsInformational(parser.incoming.statusCode)) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
freeParser(parser, req, socket);
}
}

// client
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what this comment stands for.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not either. I was following what looked like existing convention in this source file. Removing.

function statusIsInformational(status) {
// 100 (Continue) RFC7231 Section 6.2.1
// 102 (Processing) RFC2518
// 103 (Early Hints) RFC8297
// 104-199 (Unassigned)
return (status < 200 && status >= 100 && status !== 101);
}

// client
function parserOnIncomingClient(res, shouldKeepAlive) {
Expand Down Expand Up @@ -486,10 +496,24 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
return 2; // Skip body and treat as Upgrade.
}

if (res.statusCode === 100) {
// restart the parser, as this is a continue message.
// Responses to HEAD requests are crazy.
// HEAD responses aren't allowed to have an entity-body
// but *can* have a content-length which actually corresponds
// to the content-length of the entity-body had the request
// been a GET.
var isHeadResponse = req.method === 'HEAD';
debug('AGENT isHeadResponse', isHeadResponse);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this a while ago, see #17806 (comment). Can you do the same?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this got back in / was not removed?


if (statusIsInformational(res.statusCode)) {
// Restart the parser, as this is a 1xx informational message.
req.res = null; // Clear res so that we don't hit double-responses.
req.emit('continue');
// Maintain compatibility by sending 100-specific events
if (res.statusCode === 100) {
req.emit('continue');
}
// Send information events to all 1xx responses except 101 Upgrade.
req.emit('information', { statusCode: res.statusCode });

return 1; // Skip body but don't treat as Upgrade.
}

Expand Down
4 changes: 4 additions & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ ServerResponse.prototype.writeContinue = function writeContinue(cb) {
this._sent100 = true;
};

ServerResponse.prototype.writeProcessing = function writeProcessing(cb) {
this._writeRaw(`HTTP/1.1 102 Processing${CRLF}${CRLF}`, 'ascii', cb);
};

ServerResponse.prototype._implicitHeader = function _implicitHeader() {
this.writeHead(this.statusCode);
};
Expand Down
51 changes: 51 additions & 0 deletions test/parallel/test-http-information-processing.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
'use strict';
require('../common');
const assert = require('assert');
const http = require('http');

const test_res_body = 'other stuff!\n';
let processing_count = 0;

const server = http.createServer((req, res) => {
console.error('Server sending informational message #1...');
res.writeProcessing();
console.error('Server sending informational message #2...');
res.writeProcessing();
console.error('Server sending full response...');
res.writeHead(200, {
'Content-Type': 'text/plain',
'ABCD': '1'
});
res.end(test_res_body);
});

server.listen(0, function() {
const req = http.request({
port: this.address().port,
path: '/world'
});
req.end();
console.error('Client sending request...');

let body = '';

req.on('information', function(res) {
console.error('Client got 102 Processing...');
processing_count++;
});

req.on('response', function(res) {
assert.strictEqual(processing_count, 2,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use common/countdown.js. That would be more intuitive.

'Full response received before all 102 Processing');
assert.strictEqual(200, res.statusCode,
`Final status code was ${res.statusCode}, not 200.`);
res.setEncoding('utf8');
res.on('data', function(chunk) { body += chunk; });
res.on('end', function() {
console.error('Got full response.');
assert.strictEqual(body, test_res_body, 'Response body doesn\'t match.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having any message will probably be even better because it will show what values got in.

assert.ok('abcd' in res.headers, 'Response headers missing.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving out the error message is actually better because in case of an error it would say:
The expression assert.ok('abcd' in res.headers) evaluated to a falsy value.

server.close();
});
});
});