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

[v8.x backport] http: fix undefined error in parser event #22880

Closed
wants to merge 1 commit into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Sep 15, 2018

Original PR: #20029
Refs: #22857

The current check for socket.server[kIncomingMessage] does not
account for the possibility of a socket.server that doesn't
have that property defined. Fix it.

PR-URL: #20029
Fixes: #19231
Reviewed-By: Ben Noordhuis [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: Trivikram Kamat [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Ruben Bridgewater [email protected]
Reviewed-By: Khaidi Chu [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The current check for socket.server[kIncomingMessage] does not
account for the possibility of a socket.server that doesn't
have that property defined. Fix it.

PR-URL: nodejs#20029
Fixes: nodejs#19231
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. v8.x labels Sep 15, 2018
@trivikr
Copy link
Member Author

trivikr commented Sep 15, 2018

@MylesBorins
Copy link
Contributor

One more try with CI: https://ci.nodejs.org/job/node-test-pull-request/17437/

@MylesBorins
Copy link
Contributor

potentially related fail in ubuntu 1404

17:08:57 not ok 541 parallel/test-fs-read-stream-encoding
17:08:57   ---
17:08:57   duration_ms: 0.470
17:08:57   severity: crashed
17:08:57   exitcode: -6
17:08:57   stack: |-
17:08:57   ...

@BethGriggs
Copy link
Member

@BethGriggs
Copy link
Member

@MylesBorins
Copy link
Contributor

MylesBorins pushed a commit that referenced this pull request Oct 31, 2018
The current check for socket.server[kIncomingMessage] does not
account for the possibility of a socket.server that doesn't
have that property defined. Fix it.

Backport-PR-URL: #22880
PR-URL: #20029
Fixes: #19231
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
@MylesBorins
Copy link
Contributor

landed in e3bddee

@trivikr trivikr deleted the backport-20029-to-v8.x branch October 31, 2018 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants