UNIX Socket URL Support #516

Merged
merged 3 commits into from Feb 9, 2014

Projects

None yet

4 participants

@lyuzashi
Contributor

Added support for http requests over unix sockets via the unix:// protocol name.
Uses net.connect to discover the listening socket inside a full url and sends the remaining path using existing request methods.

Summary for use is added to README.md

The asynchronous socket listener discovery loop fails with an Error after 1000 ticks to prevent hangs, successful connections appear to happen within a couple of ticks.

@mikeal
Member
mikeal commented Apr 16, 2013

this doesn't merge cleanly :(

@lyuzashi
Contributor

Hi @mikeal I've fixed my fork to merge cleanly with 638c568. Sorry about that

@mikeal
Member
mikeal commented Apr 24, 2013

i'm going to need some time to review this. i want to bring it in, but its a big change and i'll need to find a good time to review it all.

@lyuzashi
Contributor

Thanks for the update @mikeal, I'll try to put together a test when I get time too. I had to wrap a big chunk of init in a secondary function so the socket lookup could callback request building.
Let me know if you have any questions at all. I've got my fork running on a production app without issues.

@zheng1
zheng1 commented Jan 10, 2014

can we use unix socket now?

@apatil
apatil commented Feb 6, 2014

+1, useful PR.

@mikeal
Member
mikeal commented Feb 6, 2014

It still says it won't merge cleanly :(

@lyuzashi
Contributor
lyuzashi commented Feb 7, 2014

Hey @mikeal, looks like a lot has changed in request since I looked at this. I've reintegrated my fork into the latest version and wrote a test for basic UNIX domain socket requests.

I also double checked the fork merges cleanly, so hoping you are able to close this pull request. Thanks :)

@mikeal mikeal and 1 other commented on an outdated diff Feb 7, 2014
@@ -241,6 +242,9 @@ Request.prototype.init = function (options) {
}
}
+ self._buildRequest = function(){
@mikeal
mikeal Feb 7, 2014 Member

should alter the indentation if you're putting this whole thing in a function.

@lyuzashi
lyuzashi Feb 9, 2014 Contributor

Good point. I've indented the function. Let me know if there's anything else that needs updating.

@mikeal mikeal merged commit e2a14cc into request:master Feb 9, 2014

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment