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

Doesn't get any responce from https.get on specific https website #4889

Closed
Knighton910 opened this issue Jan 26, 2016 · 18 comments
Closed

Doesn't get any responce from https.get on specific https website #4889

Knighton910 opened this issue Jan 26, 2016 · 18 comments
Assignees
Labels
https Issues or PRs related to the https subsystem.

Comments

@Knighton910
Copy link

This issue originally popped up on ( nodejs/help ) nodejs/help#78

var https=require('https');

https.get({ host:'www.juvalis.de', path: '/' }, function(res) {
    res.on('data', function(d) {
        console.log('chunk');
    });

    res.on('end', function() {
        console.log('end');
    });
}).on('error', function(e) {
    console.error(e);
}).on('close',function(){
    console.log('close')
});

So i responded to it, and started helping him, but i couldn't get any where, so @thealphanerd and @bnoordhuis came to help, after help from @thealphanerd https://gist.github.com/TheAlphaNerd/af65cf4f9ef9d727d8d0 and advice from @bnoordhuis about doing git bisect. So where this gets interesting is that for TheAlphaNerd & bnoordhuis is that it worked for them, running on master , I'm running v4 so bottom line, were trying to get this issue fixed.

@MylesBorins MylesBorins self-assigned this Jan 27, 2016
@MylesBorins MylesBorins added https Issues or PRs related to the https subsystem. v4.x labels Jan 27, 2016
@MylesBorins
Copy link
Member

Thanks for posting the bug report @kelthenoble

So whatever is causing this problem is broken on v5.x and v4.x but working on master. @kelthenoble has volunteered to find the commit that is currently on master that is fixing the problem. The best solution IMHO would be to write a regression test that we can use to make sure this does not break again

@Knighton910
Copy link
Author

yeah, it's no worries. So I been feeling under the weather today, but i should be back in full swing tomorrow & i will totally work on this. 💯 ⚾

@evanlucas
Copy link
Contributor

so this doesn't work even on v4.0.0. Still looking

@vkurchatkin
Copy link
Contributor

32ac376

Server sends following headers:

Upgrade: h2c,h2
Connection: Upgrade

@evanlucas
Copy link
Contributor

@vkurchatkin I'm seeing this fail way before that though

@vkurchatkin
Copy link
Contributor

@evanlucas this commit fixes it (at least I think so)

@evanlucas
Copy link
Contributor

ahhhh my bad. That would make sense. And it is semver-major, so it isn't in anything that has been released...apologies

@vkurchatkin
Copy link
Contributor

I guess this should be backported, otherwise older versions won't be able to talk to HTTP2 enabled servers

@silverwind
Copy link
Contributor

Same root cause as #4844 I assume.

@vkurchatkin
Copy link
Contributor

@silverwind yep, closing

@silverwind
Copy link
Contributor

I'll close #4844 in favor of #4334 😉

@Knighton910
Copy link
Author

sounds good @silverwind since #4334 is further in progress. Thanks 🎱

@silverwind
Copy link
Contributor

@kelthenoble the fix for this is already in master. There are some concerns about backwards compat, but I guess if you can't wait, use a nightly.

@Knighton910
Copy link
Author

Okay so 32ac376 that's the fix right? So it's working for master but not v5?

@silverwind
Copy link
Contributor

Yeah, as it stands, the fix will release in 6.0.0 on April 30, but we might change that. Edited above message to suggest a nightly :)

@Knighton910
Copy link
Author

@silverwind Okay cool, so as far as nodejs/help#78, so my best bet is tell the guy theres a fix in progress, but for now, he would have to go with a nightly?

@silverwind
Copy link
Contributor

Yes, to root issue looks to be that Apache 2.4.18 (released Dec 14) changed the way HTTP2 support is advertised. Given the impact of this issue, I think we'll release something very soon.

@Knighton910
Copy link
Author

Alright, will do @silverwind, thanks for all of the help received guys 💯 This helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
https Issues or PRs related to the https subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants