Skip to content

http: don't emit 'finish' after 'error' - #32276

Closed
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:http-error-finish
Closed

http: don't emit 'finish' after 'error'#32276
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:http-error-finish

Conversation

@ronag

@ronagronag commented Mar 14, 2020

Copy link
Copy Markdown
Member

An edge case could emit 'finish' after 'error'.

Refs: #28710

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-botnodejs-github-bot added http Issues or PRs related to the http subsystem. stream Issues and PRs related to the stream subsystem. labels Mar 14, 2020
@ronag
ronagforce-pushed the http-error-finish branch from ace1aec to c67b545CompareMarch 14, 2020 23:55
@ronagronag removed the stream Issues and PRs related to the stream subsystem. label Mar 14, 2020
@nodejs-github-bot

nodejs-github-bot commented Mar 15, 2020

Copy link
Copy Markdown
Collaborator

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

@ronagronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 16, 2020
@nodejs-github-bot

nodejs-github-bot commented Mar 16, 2020

Copy link
Copy Markdown
Collaborator

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

@ronag

ronag commented Mar 18, 2020

Copy link
Copy Markdown
MemberAuthor

Landed in a9270dc

@ronagronag closed this Mar 18, 2020
ronag added a commit that referenced this pull request Mar 18, 2020
@ronag
http: don't emit 'finish' after 'error'
a9270dc
PR-URL: #32276
Refs: #28710
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 19, 2020
@ronag@MylesBorins
http: don't emit 'finish' after 'error'
720a2e2
PR-URL: #32276
Refs: #28710
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Mar 19, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
@ronag@MylesBorins
http: don't emit 'finish' after 'error'
ef32069
PR-URL: #32276
Refs: #28710
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targos

targos commented Apr 22, 2020

Copy link
Copy Markdown
Member

is this safe to land on v12 ?

@ronag

ronag commented Apr 22, 2020

Copy link
Copy Markdown
MemberAuthor

@targos: I think so.

targos pushed a commit that referenced this pull request Apr 22, 2020
@ronag@targos
http: don't emit 'finish' after 'error'
cf47bb9
PR-URL: #32276
Refs: #28710
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targostargos mentioned this pull request Apr 22, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@trivikrtrivikrtrivikr approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.httpIssues or PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ronag@nodejs-github-bot@targos@jasnell@trivikr