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

WS continuation frames & node > 4.0 #881

Open
pepone opened this issue Sep 17, 2015 · 12 comments
Open

WS continuation frames & node > 4.0 #881

pepone opened this issue Sep 17, 2015 · 12 comments

Comments

@pepone
Copy link

pepone commented Sep 17, 2015

After upgrade to nodejs 4.0.0 I see that the proxy is not forwarding WS trafict that contains continuation frames, tested with nodejs 4.0.0 and 4.1.0 and see the same behavior with both. Everything works fine with node 0.12.7.

Is there any incompatibility issues with node >= 4.0?

@glortho
Copy link

glortho commented Oct 28, 2015

We're seeing the same issue. Works fine on 0.12.7 but not in iojs or node 4.x.

@indexzero
Copy link
Contributor

@pepone @jedverity seems like it. Could you provide a complete console output of what you're seeing? Is there a reproducible test case that you've found? This will help us narrow down the problem.

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 28, 2015

yea this would be super helpful in troubleshooting. I know things related to http changed in iojs and node@4 so this could make sense.

@glortho
Copy link

glortho commented Oct 28, 2015

Thanks @indexzero @jcrugzz - we're working on putting together a reproducible test case for you right now. Stay tuned.

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 28, 2015

@jedverity awesome :). A failing test PR is my favorite thing

@glortho
Copy link

glortho commented Oct 28, 2015

@jcrugzz I'll see what I can do about a failing test. In the meantime, here is a reproducible set-up: https://github.com/jedverity/http-proxy-test

@glortho
Copy link

glortho commented Oct 28, 2015

Think I have a (kludgy) test for you @jcrugzz @indexzero . Coming in 5 mins.

@glortho
Copy link

glortho commented Oct 30, 2015

@indexzero @jcrugzz @pepone I've confirmed this is fixed in node v5. See http fixes section here: http://nodejs.org/en/blog/release/v5.0.0/

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 30, 2015

@jedverity hmm well we should see if whatever fix can be backported if it is not a breaking change. Have you tried it in 4.2.1? That is the LTS release so we should get this fixed in that release

@glortho
Copy link

glortho commented Oct 30, 2015

@jcrugzz I did try it in 4.2.1 and no dice. Same results as other 4.x versions. I assumed this was the fix but haven't evaluated closely: nodejs/node#3342

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 30, 2015

@jedverity i think you might be right, looks like its on target for 4.2.2 if you are correct. See if you can test the 4.2.2 release candidate for confirmation. Either way your tests are still good :). Will add them anyway

@glortho
Copy link

glortho commented Nov 5, 2015

@jcrugzz Confirmed 4.2.2 works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants