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

Defer piping of options.payload until socket connection #223

Merged
merged 2 commits into from Jul 24, 2018

Conversation

@ggoodman
Copy link
Contributor

ggoodman commented Jul 23, 2018

Fixes #222.

@ggoodman

This comment has been minimized.

Copy link
Contributor Author

ggoodman commented Jul 23, 2018

I've investigated why this is failing on travis against node@10. It turns out that a change was introduced in the order of stream events in 10.2 that will be backed out and deferred until 11.x.

See: nodejs/node#20611 (comment) and the work to revert this change: nodejs/node#21809.

@geek geek self-assigned this Jul 23, 2018
@geek geek self-requested a review Jul 23, 2018
@geek geek added feature bug and removed feature labels Jul 23, 2018
@geek geek added this to the 14.0.3 milestone Jul 24, 2018
@geek geek merged commit 1315545 into hapijs:master Jul 24, 2018
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 24, 2018

@ggoodman Going to look into potential workarounds for the node 10 regression before publishing.

@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 24, 2018

@ggoodman I do have a workaround, but it feels hacky for this special case... going to wait until the next 10.x release, which should include the regression fix.

@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 24, 2018

@ggoodman if you are curious on the workaround #224

@ggoodman ggoodman mentioned this pull request Jul 24, 2018
0 of 3 tasks complete
@ggoodman ggoodman deleted the ggoodman:fix-pipe-after-socket-connect branch Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.