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.

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

[v6.x] http: fix error check in Execute() #25939

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 5, 2019

Refs: #24738
Fixes: #25858

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

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

@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Feb 5, 2019
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. v6.x labels Feb 5, 2019
@mscdex mscdex mentioned this pull request Feb 5, 2019
3 tasks
@addaleax addaleax changed the title http: fix error check in Execute() [v6.x] http: fix error check in Execute() Feb 9, 2019
@mscdex mscdex force-pushed the v6-http-fix-parser-execute-error-check branch from 084ccb3 to 41f8dd5 Compare February 28, 2019 16:57
@mscdex
Copy link
Contributor Author

mscdex commented Mar 6, 2019

ping @nodejs/http

@lpinca
Copy link
Member

lpinca commented Mar 9, 2019

@mscdex I think this needs a rebase.

@mscdex mscdex force-pushed the v6-http-fix-parser-execute-error-check branch from 41f8dd5 to 858004a Compare March 9, 2019 14:52
@mscdex
Copy link
Contributor Author

mscdex commented Mar 9, 2019

@lpinca Done.

@lpinca
Copy link
Member

lpinca commented Mar 11, 2019

@nodejs/lts @MylesBorins

@BethGriggs
Copy link
Member

BethGriggs commented Mar 11, 2019

BethGriggs pushed a commit that referenced this pull request Mar 11, 2019
Refs: #24738
Fixes: #25858

PR-URL: #25939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
@BethGriggs
Copy link
Member

Landed in 8080a9b

@BethGriggs BethGriggs closed this Mar 11, 2019
@mscdex mscdex deleted the v6-http-fix-parser-execute-error-check branch March 11, 2019 16:38
@BethGriggs BethGriggs mentioned this pull request Mar 15, 2019
BethGriggs added a commit that referenced this pull request Mar 15, 2019
Notable changes:

* **http**:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)
BethGriggs added a commit that referenced this pull request Mar 19, 2019
Notable changes:

* **http**:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)
BethGriggs added a commit that referenced this pull request Apr 2, 2019
Notable changes:

- http:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)
BethGriggs added a commit that referenced this pull request Apr 3, 2019
Notable changes:

- http:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)

PR-URL: #26684
BethGriggs added a commit that referenced this pull request Apr 3, 2019
Notable changes:

- http:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)

PR-URL: #26684
BethGriggs added a commit that referenced this pull request Apr 3, 2019
Notable changes:

- http:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)

PR-URL: #26684
BethGriggs added a commit that referenced this pull request Apr 4, 2019
Notable changes:

- http:
  - fix error check in `Execute()` (Brian White)
    [#25939](#25939)

PR-URL: #26684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants