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

http2: Change in respondWithFile api not properly reflected in the docs #15390

Closed
akc42 opened this issue Sep 13, 2017 · 1 comment
Closed

http2: Change in respondWithFile api not properly reflected in the docs #15390

akc42 opened this issue Sep 13, 2017 · 1 comment
Labels
doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem.

Comments

@akc42
Copy link

akc42 commented Sep 13, 2017 •
edited
Loading

The respondWithFile api has changed between 8.4.0 and 8.5.0 with the addition of the onError option. This is important, because a simple response.stream.on('error'... no longer works after this change because the steam gets destroyed on an error (in particular on ENOENT) if onError doesn't exist so any attempt to resend a different file then crashes with stream undefined.

However the docs, although creating an onError function in the example then doesn't actually use it on the call to respondWithFile
@mscdex mscdex added http2 Issues or PRs related to the http2 subsystem. doc Issues and PRs related to the documentations. labels Sep 13, 2017
@antoine-amara
Copy link
Contributor

antoine-amara commented Sep 18, 2017 •
edited
Loading

I can update http2 documentation, I found where I have to change the text. I will submit a PR soon with few updates.

@antoine-amara antoine-amara mentioned this issue Sep 20, 2017
Closed
4 tasks
antoine-amara added a commit to antoine-amara/node that referenced this issue Sep 26, 2017
@antoine-amara
doc: fix mistake in http2stream.respondWithFile.
42c244d 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

Fixes: nodejs#15390
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Sep 28, 2017
@antoine-amara @MylesBorins
doc: fix mistake in http2stream.respondWithFile.
2ed0d1d 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: nodejs#15501
Fixes: nodejs#15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Sep 29, 2017
@antoine-amara @MylesBorins
doc: fix mistake in http2stream.respondWithFile.
ea2210e 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit to addaleax/ayo that referenced this issue Sep 30, 2017
@antoine-amara @addaleax
doc: fix mistake in http2stream.respondWithFile.
8e272ac 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: nodejs/node#15501
Fixes: nodejs/node#15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Oct 3, 2017
@antoine-amara @MylesBorins
doc: fix mistake in http2stream.respondWithFile.
70f60c9 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Oct 3, 2017
@antoine-amara @MylesBorins
doc: fix mistake in http2stream.respondWithFile.
22b2d1a 
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: fix mistake in http2stream.respondWithFile. antoine-amara/node
3 participants
@mscdex @akc42 @antoine-amara