http: enable setHeader for call chaining

[PoojaDurgad]

Make response.setHeader return the response object itself
so that multiple header setting can be chained.

Fixes: #33148

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[mscdex]

The targeted subsystem can just be http in the commit message instead of lib,http since http is the only thing being changed here.

[lpinca]

Can you please add a test?

[lpinca]

I'm fine with the test file as is but it could be simplified to something like

'use strict';
require('../common');
const assert = require('assert');
const { ServerResponse } = require('http');

const response = new ServerResponse({
  method: 'GET',
  httpVersionMajor: 1,
  httpVersionMinor: 1
});

assert.strictEqual(response.setHeader('foo', 'bar'), response);

or just a single assertion like the one above in an already existing test where response.setHeader() is used.

There is no need to start a server and chaining is a logical consequence of returning the same object.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/34072/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/34079/

[Trott]

Nit on the commit message. Maybe this is a little more clear?: http: enable call chaining with setHeader() Or if you don't like that, maybe this?: http: enable setHeader() call chaining

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/34085/

[Trott]

Will this automatically add this for https as well? If so, should we add a test? If not, should we also add it to https?

[Trott]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/680/

[PoojaDurgad]

Will this automatically add this for https as well? If so, should we add a test? If not, should we also add it to https?

@Trott - This behavior will be added to the https and need a test I guess . I think this feature can also be added to the http2 module too!!

[Trott]

Benchmark results show a small performance hit in some cases. Probably acceptable? @nodejs/http

07:04:12                                                                    confidence improvement accuracy (*)   (**)  (***)
07:04:12  http/set-header.js duration=5 res='normal' benchmarker='wrk'                     -2.36 %       ±4.51% ±6.01% ±7.84%
07:04:12  http/set-header.js duration=5 res='setHeader' benchmarker='wrk'            *     -3.48 %       ±3.17% ±4.22% ±5.49%
07:04:12  http/set-header.js duration=5 res='setHeaderWH' benchmarker='wrk'                -2.83 %       ±3.86% ±5.14% ±6.71%
07:04:12  http/set_header.js n=1000000 value='Connection'                                  -2.96 %       ±4.18% ±5.62% ±7.44%
07:04:12  http/set_header.js n=1000000 value='Content-Length'                              -0.92 %       ±2.31% ±3.08% ±4.03%
07:04:12  http/set_header.js n=1000000 value='Content-Type'                                 0.05 %       ±1.31% ±1.74% ±2.27%
07:04:12  http/set_header.js n=1000000 value='Set-Cookie'                                   0.75 %       ±1.57% ±2.09% ±2.72%
07:04:12  http/set_header.js n=1000000 value='Transfer-Encoding'                           -0.75 %       ±3.16% ±4.22% ±5.54%
07:04:12  http/set_header.js n=1000000 value='Vary'                                  *     -2.19 %       ±1.95% ±2.61% ±3.41%
07:04:12  http/set_header.js n=1000000 value='X-Powered-By'                                 0.95 %       ±1.86% ±2.48% ±3.23%
07:04:12 
07:04:12 Be aware that when doing many comparisons the risk of a false-positive
07:04:12 result increases. In this case there are 10 comparisons, you can thus
07:04:12 expect the following amount of false-positive results:
07:04:12   0.50 false positives, when considering a   5% risk acceptance (*, **, ***),
07:04:12   0.10 false positives, when considering a   1% risk acceptance (**, ***),
07:04:12   0.01 false positives, when considering a 0.1% risk acceptance (***)

[PoojaDurgad]

is there anything pending on this PR? if yes let me know .

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/34595/

[Trott]

Landed in dedd061