Skip to content

http2: use original error for cancelling pending streams#18988

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:http2-pending-cancel-keep-error
Closed

http2: use original error for cancelling pending streams#18988
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:http2-pending-cancel-keep-error

Conversation

@addaleax
Copy link
Copy Markdown
Member

@addaleax addaleax commented Feb 25, 2018

Previously, if session.destroy() was called with an error object,
the information contained in it would be discarded and a generic
ERR_HTTP2_STREAM_CANCEL would be used for all pending streams.

Instead, make the information from the original error object
available.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs/http2
@addaleax
http2: use original error for cancelling pending streams
97cb4a7 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.
@nodejs-github-bot nodejs-github-bot added dont-land-on-v4.x http2 Issues or PRs related to the http2 subsystem. labels Feb 25, 2018
@addaleax
Copy link
Copy Markdown
Member Author

addaleax commented Feb 25, 2018

CI: https://ci.nodejs.org/job/node-test-commit/16489/
mcollina
mcollina approved these changes Feb 26, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 1, 2018
@addaleax
Copy link
Copy Markdown
Member Author

addaleax commented Mar 2, 2018

Landed in c197c78
@addaleax addaleax closed this Mar 2, 2018
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 2, 2018
addaleax added a commit that referenced this pull request Mar 2, 2018
@addaleax
http2: use original error for cancelling pending streams
c197c78 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

PR-URL: #18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax added a commit to addaleax/node that referenced this pull request Mar 5, 2018
@addaleax
http2: use original error for cancelling pending streams
aa0fca9 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

PR-URL: nodejs#18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Mar 6, 2018
Merged
kjin pushed a commit to kjin/node that referenced this pull request May 1, 2018
@addaleax @kjin
http2: use original error for cancelling pending streams
78b6542 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

PR-URL: nodejs#18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 2, 2018
@addaleax @MylesBorins
http2: use original error for cancelling pending streams
0379350 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

Backport-PR-URL: #20456
PR-URL: #18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Merged
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@addaleax @MayaLekova
http2: use original error for cancelling pending streams
ec37c17 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

PR-URL: nodejs#18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
@addaleax @MylesBorins
http2: use original error for cancelling pending streams
81c08a1 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

Backport-PR-URL: #20456
PR-URL: #18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
@addaleax @MylesBorins
http2: use original error for cancelling pending streams
7e4a9c9 
Previously, if `session.destroy()` was called with an error object,
the information contained in it would be discarded and a generic
`ERR_HTTP2_STREAM_CANCEL` would be used for all pending streams.

Instead, make the information from the original error object
available.

Backport-PR-URL: #20456
PR-URL: #18988
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.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

Labels

http2 Issues or PRs related to the http2 subsystem.

7 participants

@addaleax @mcollina @jasnell @lpinca @cjihrig @BridgeAR @nodejs-github-bot