Skip to content

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 info@bnoordhuis.nl
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Trivikram Kamat trivikr.dev@gmail.com
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
Reviewed-By: Khaidi Chu i@2333.moe

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 <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Khaidi Chu <i@2333.moe>
@nodejs-github-bot
Copy link
Collaborator

@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 <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Khaidi Chu <i@2333.moe>
@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