Makes http(2) response.writehead return this by qubyte · Pull Request #25974 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

qubyte wants to merge7 commits intonodejs:masterfromqubyte:http-response-writehead-return-this

Conversation10 Commits7 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Addresses #25935.

I chose in the end to go with the lightest touch and only update writeHead (this is the most useful to change in my experience). I'm happy to extend this to res.setHeader too, but it may suggest updating other similar methods (flushHeaders, removeHeader, etc.)

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

@@ -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').

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.

Addresses PR feedback.

Co-Authored-By: qubyte mark.s.everitt@gmail.com

Addresses PR feedback.

Co-Authored-By: qubyte mark.s.everitt@gmail.com

Choose a reason for hiding this comment

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

Thank you. Some doc nits.

Addresses PR feedback.

Co-Authored-By: qubyte mark.s.everitt@gmail.com

Addresses PR feedback.

Co-Authored-By: qubyte mark.s.everitt@gmail.com

Addresses PR feedback.

Co-Authored-By: qubyte mark.s.everitt@gmail.com

addaleax added the semver-minor

PRs that contain new features and should be released in the next minor version.

label

Feb 9, 2019

addaleax pushed a commit that referenced this pull request

Feb 9, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

addaleax pushed a commit that referenced this pull request

Feb 9, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

targos pushed a commit that referenced this pull request

Feb 10, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

targos pushed a commit that referenced this pull request

Feb 10, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

This was referenced

Feb 15, 2019

qubyte deleted the http-response-writehead-return-this branch

February 27, 2019 22:54

BethGriggs pushed a commit that referenced this pull request

Oct 7, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

BethGriggs pushed a commit that referenced this pull request

Oct 7, 2019

Fixes: #25935

PR-URL: #25974 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Luigi Pinca luigipinca@gmail.com

BethGriggs added a commit that referenced this pull request

Oct 22, 2019

Notable changes:

crypto:
- add support for chacha20-poly1305 for AEAD (chux0519) #24081
- increase maxmem range from 32 to 53 bits (Tobias Nießen) #28799
deps:
- update npm to 6.11.3 (claudiahdz) #29430
- upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
dns:
- remove dns.promises experimental warning (cjihrig) #26592
fs:
- remove experimental warning for fs.promises (Anna Henningsen) #26581
http:
- makes response.writeHead return the response (Mark S. Everitt) #25974
http2:
- makes response.writeHead return the response (Mark S. Everitt) #25974
n-api:
- make func argument of napi_create_threadsafe_function optional (legendecas) #27791
- mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
- implement date object (Jarrod Connolly) #25917
process:
- add --unhandled-rejections flag (Ruben Bridgewater) #26599
stream:
- implement Readable.from async iterator utility (Guy Bedford) #27660
- make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

BethGriggs added a commit that referenced this pull request

Oct 22, 2019

Notable changes:

crypto:
- add support for chacha20-poly1305 for AEAD (chux0519) #24081
- increase maxmem range from 32 to 53 bits (Tobias Nießen) #28799
deps:
- update npm to 6.11.3 (claudiahdz) #29430
- upgrade openssl sources to 1.1.1d (Sam Roberts) #29921
dns:
- remove dns.promises experimental warning (cjihrig) #26592
fs:
- remove experimental warning for fs.promises (Anna Henningsen) #26581
http:
- makes response.writeHead return the response (Mark S. Everitt) #25974
http2:
- makes response.writeHead return the response (Mark S. Everitt) #25974
n-api:
- make func argument of napi_create_threadsafe_function optional (legendecas) #27791
- mark version 5 N-APIs as stable (Gabriel Schulhof) #29401
- implement date object (Jarrod Connolly) #25917
process:
- add --unhandled-rejections flag (Ruben Bridgewater) #26599
stream:
- implement Readable.from async iterator utility (Guy Bedford) #27660
- make Symbol.asyncIterator support stable (Matteo Collina) #26989

PR-URL: #29875

Labels

http

Issues or PRs related to the http subsystem.

http2

Issues or PRs related to the http2 subsystem.

semver-minor

PRs that contain new features and should be released in the next minor version.