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.

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

Makes http(2) response.writehead return this #25974

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
15 changes: 12 additions & 3 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,9 @@ the request body should be sent. See the [`'checkContinue'`][] event on
<!-- YAML
added: v0.1.30
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25974
description: Return `this` from `writeHead` to allow chaining with `end`.
qubyte marked this conversation as resolved.
Show resolved Hide resolved
- version: v5.11.0, v4.4.5
pr-url: https://github.com/nodejs/node/pull/6291
description: A `RangeError` is thrown if `statusCode` is not a number in
Expand All @@ -1455,17 +1458,23 @@ changes:
* `statusCode` {number}
* `statusMessage` {string}
* `headers` {Object}
* Returns: {http.ServerResponse}

Sends a response header to the request. The status code is a 3-digit HTTP
status code, like `404`. The last argument, `headers`, are the response headers.
Optionally one can give a human-readable `statusMessage` as the second
argument.

Returns a reference to the `ServerResponse`, so that calls can be chained.

```js
const body = 'hello world';
response.writeHead(200, {
'Content-Length': Buffer.byteLength(body),
'Content-Type': 'text/plain' });
response
.writeHead(200, {
'Content-Length': Buffer.byteLength(body),
'Content-Type': 'text/plain'
})
.end(body);
```

This method must only be called once on a message and it must
Expand Down
7 changes: 7 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -3283,15 +3283,22 @@ should be sent. See the [`'checkContinue'`][] event on `Http2Server` and
#### response.writeHead(statusCode[, statusMessage][, headers])
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25974
description: Return `this` from `writeHead` to allow chaining with `end`.
qubyte marked this conversation as resolved.
Show resolved Hide resolved
-->

* `statusCode` {number}
* `statusMessage` {string}
* `headers` {Object}
* Returns: {http2.Http2ServerResponse}

Sends a response header to the request. The status code is a 3-digit HTTP
status code, like `404`. The last argument, `headers`, are the response headers.

Returns a reference to the `ServerResponse`, so that calls can be chained.
qubyte marked this conversation as resolved.
Show resolved Hide resolved

For compatibility with [HTTP/1][], a human-readable `statusMessage` may be
passed as the second argument. However, because the `statusMessage` has no
meaning within HTTP/2, the argument will have no effect and a process warning
Expand Down
2 changes: 2 additions & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ function writeHead(statusCode, reason, obj) {
}

this._storeHeader(statusLine, headers);

return this;
}

// Docs-only deprecated: DEP0063
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,10 +568,8 @@ class Http2ServerResponse extends Stream {
if (this[kStream].headersSent)
throw new ERR_HTTP2_HEADERS_SENT();

// If the stream is destroyed, we return false,
// like require('http').
Copy link
Contributor Author

@qubyte qubyte Feb 7, 2019

Choose a reason for hiding this comment

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

I couldn't find the code which this refers to. The HTTP version of writeHead appears to have no explicit returns.

if (this.stream.destroyed)
return false;
return this;

if (typeof statusMessage === 'string')
statusMessageWarn();
Expand All @@ -596,6 +594,8 @@ class Http2ServerResponse extends Stream {

state.statusCode = statusCode;
this[kBeginSend]();

return this;
}

write(chunk, encoding, cb) {
Expand Down
22 changes: 22 additions & 0 deletions test/parallel/test-http-response-writehead-returns-this.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
require('../common');
const http = require('http');
const assert = require('assert');

const server = http.createServer((req, res) => {
res.writeHead(200, { 'a-header': 'a-header-value' }).end('abc');
});

server.listen(0, () => {
http.get({ port: server.address().port }, (res) => {
assert.strictEqual(res.headers['a-header'], 'a-header-value');

const chunks = [];

res.on('data', (chunk) => chunks.push(chunk));
res.on('end', () => {
assert.strictEqual(Buffer.concat(chunks).toString(), 'abc');
server.close();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ const server = h2.createServer();
server.listen(0, common.mustCall(() => {
const port = server.address().port;
server.once('request', common.mustCall((request, response) => {
response.writeHead(200, [
const returnVal = response.writeHead(200, [
['foo', 'bar'],
['ABC', 123]
]);
assert.strictEqual(returnVal, response);
response.end(common.mustCall(() => { server.close(); }));
}));

Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-http2-compat-serverresponse-writehead.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ server.listen(0, common.mustCall(function() {
const port = server.address().port;
server.once('request', common.mustCall(function(request, response) {
response.setHeader('foo-bar', 'def456');
response.writeHead(418, { 'foo-bar': 'abc123' }); // Override

// Override
const returnVal = response.writeHead(418, { 'foo-bar': 'abc123' });

assert.strictEqual(returnVal, response);

common.expectsError(() => { response.writeHead(300); }, {
code: 'ERR_HTTP2_HEADERS_SENT'
Expand Down