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

Adding support for UNIX Sockets #1070

Merged
merged 1 commit into from Feb 17, 2018

Conversation

Projects
None yet
7 participants
@Starfox64
Copy link
Contributor

Starfox64 commented Sep 4, 2017

Adds support for sending requests to a server running on a UNIX Socket (.sock files).
This will only work in Node.js with the http adapter.

Closes #975

@coveralls

This comment has been minimized.

Copy link

coveralls commented Sep 4, 2017

Coverage Status

Coverage remained the same at 93.812% when pulling 85a48b2 on Starfox64:master into 07a7b7c on mzabriskie:master.

@sam3d

This comment has been minimized.

Copy link

sam3d commented Oct 26, 2017

LGTM 👍

Would love to have this feature. Any progress or updates on this? Looks like a pretty straightforward patch and I've tested locally with success.

/cc @nickuraltsev

@meyt

This comment has been minimized.

Copy link

meyt commented Jan 20, 2018

@nickuraltsev, @rubennorte please consider about this patch.

@wood1986

This comment has been minimized.

Copy link

wood1986 commented Jan 28, 2018

Any updates about this PR?

@acupofspirt

This comment has been minimized.

Copy link

acupofspirt commented Feb 15, 2018

It is awesome pr.
When you have Vue/React SSR, your node process make requests to endpoint like http://foo.bar/api/smth which is processed by, say, ruby process. Even if node and ruby hosted on the same environment, there will be full dns lookup, rtt and other boring words.
If we make request to nix socket we can save much time for all that network stuff.

@rubennorte please look at pr

@sam3d

sam3d approved these changes Feb 15, 2018

@sam3d

This comment has been minimized.

Copy link

sam3d commented Feb 16, 2018

@emilyemorehouse Don't suppose you could take a look at this? 😄

@emilyemorehouse

This comment has been minimized.

Copy link
Member

emilyemorehouse commented Feb 16, 2018

The only potential issue I'm seeing is if a user sets both a socket and a proxy. The HTTP module says you should only use one or the other (see here: https://nodejs.org/api/http.html#http_http_request_options_callback) since setting a proxy winds up setting the hostname/port/etc.

I'd be fine with adding a note to the README to let users know they should only use either a socket or proxy, and what happens if they try to use both. From what I could see, if you do specify a proxy and a socket, it just uses the socket. @sam3d could you verify this since you have a test environment?

@sam3d

This comment has been minimized.

Copy link

sam3d commented Feb 16, 2018

Afraid I won't be able to get to it until tomorrow, so if somebody else can verify this behaviour faster than that feel free to! Otherwise I'll report back tomorrow whether there's any erroneous or unexpected behaviour.

Thank for you for taking a look, will get back to you soon!

@emilyemorehouse

This comment has been minimized.

Copy link
Member

emilyemorehouse commented Feb 16, 2018

@sam3d How did you set up the socket to test? I toyed around with either socat or a TCP server and am afraid I don't know nearly enough about sockets to get it working successfully!

@sam3d

This comment has been minimized.

Copy link

sam3d commented Feb 16, 2018

I actually used the express library! You can just use a filesystem path instead of a port to create a new socket:

const express = require("express");
const app = express();
const SOCK = "/var/run/test.sock";

app.get("/", (req, res) => {
    res.send("Hello, world!");
});

app.listen(SOCK, err => {
    if (err) throw err;
    console.log(`Listening on socket ${SOCK}`);
});

Then query the socket on /var/run/test.sock.

@emilyemorehouse

This comment has been minimized.

Copy link
Member

emilyemorehouse commented Feb 17, 2018

Awesome, thanks @sam3d! That was super helpful.

My assumption was correct, it does default to the socket if you provide a proxy.

I'll actually handle updating the README and adding a test for this in a separate PR to keep this one moving.

Thanks, all!

@emilyemorehouse emilyemorehouse merged commit ccc7889 into axios:master Feb 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

emilyemorehouse added a commit to emilyemorehouse/axios that referenced this pull request Feb 17, 2018

Follow up to axios#1070:
- Adding information in README for socketPath when used with a proxy
- Adding an HTTP test for socketPath option

emilyemorehouse added a commit that referenced this pull request Feb 17, 2018

Merge pull request #1366 from emilyemorehouse/fix/1070
Follow up to #1070 - adding documentation and tests

jimthedev added a commit to commitizen/cz-cli that referenced this pull request May 24, 2018

chore(deps): update dependency axios to v0.18.0 (#488)
This Pull Request updates dependency [axios](https://github.com/axios/axios) from `v0.15.2` to `v0.18.0`



<details>
<summary>Release Notes</summary>

### [`v0.18.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0180-Feb-19-2018)
[Compare Source](axios/axios@v0.17.1...v0.18.0)
- Adding support for UNIX Sockets when running with Node.js ([#&#8203;1070](`axios/axios#1070))
- Fixing typings ([#&#8203;1177](`axios/axios#1177)):
    - AxiosRequestConfig.proxy: allows type false
    - AxiosProxyConfig: added auth field
- Adding function signature in AxiosInstance interface so AxiosInstance can be invoked ([#&#8203;1192](`axios/axios#1192), [#&#8203;1254](`axios/axios#1254))
- Allowing maxContentLength to pass through to redirected calls as maxBodyLength in follow-redirects config ([#&#8203;1287](`axios/axios#1287))
- Fixing configuration when using an instance - method can now be set ([#&#8203;1342](`axios/axios#1342))

---

### [`v0.17.1`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0171-Nov-11-2017)
[Compare Source](axios/axios@v0.17.0...v0.17.1)
- Fixing issue with web workers ([#&#8203;1160](`axios/axios#1160))
- Allowing overriding transport ([#&#8203;1080](`axios/axios#1080))
- Updating TypeScript typings ([#&#8203;1165](`axios/axios#1165), [#&#8203;1125](`axios/axios#1125), [#&#8203;1131](`axios/axios#1131))

---

### [`v0.17.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0170-Oct-21-2017)
[Compare Source](axios/axios@v0.16.2...v0.17.0)
- **BREAKING** Fixing issue with `baseURL` and interceptors ([#&#8203;950](`axios/axios#950))
- **BREAKING** Improving handing of duplicate headers ([#&#8203;874](`axios/axios#874))
- Adding support for disabling proxies ([#&#8203;691](`axios/axios#691))
- Updating TypeScript typings with generic type parameters ([#&#8203;1061](`axios/axios#1061))

---

### [`v0.16.2`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0162-Jun-3-2017)
[Compare Source](axios/axios@v0.16.1...v0.16.2)
- Fixing issue with including `buffer` in bundle ([#&#8203;887](`axios/axios#887))
- Including underlying request in errors ([#&#8203;830](`axios/axios#830))
- Convert `method` to lowercase ([#&#8203;930](`axios/axios#930))

---

### [`v0.16.1`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0161-Apr-8-2017)
[Compare Source](axios/axios@v0.16.0...v0.16.1)
- Improving HTTP adapter to return last request in case of redirects ([#&#8203;828](`axios/axios#828))
- Updating `follow-redirects` dependency ([#&#8203;829](`axios/axios#829))
- Adding support for passing `Buffer` in node ([#&#8203;773](`axios/axios#773))

---

### [`v0.16.0`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0160-Mar-31-2017)
[Compare Source](axios/axios@v0.15.3...v0.16.0)
- **BREAKING** Removing `Promise` from axios typings in favor of built-in type declarations ([#&#8203;480](`axios/axios#480))
- Adding `options` shortcut method ([#&#8203;461](`axios/axios#461))
- Fixing issue with using `responseType: 'json'` in browsers incompatible with XHR Level 2 ([#&#8203;654](`axios/axios#654))
- Improving React Native detection ([#&#8203;731](`axios/axios#731))
- Fixing `combineURLs` to support empty `relativeURL` ([#&#8203;581](`axios/axios#581))
- Removing `PROTECTION_PREFIX` support ([#&#8203;561](`axios/axios#561))

---

### [`v0.15.3`](https://github.com/axios/axios/blob/master/CHANGELOG.md#&#8203;0153-Nov-27-2016)
[Compare Source](axios/axios@v0.15.2...v0.15.3)
- Fixing issue with custom instances and global defaults ([#&#8203;443](`axios/axios#443))
- Renaming `axios.d.ts` to `index.d.ts` ([#&#8203;519](`axios/axios#519))
- Adding `get`, `head`, and `delete` to `defaults.headers` ([#&#8203;509](`axios/axios#509))
- Fixing issue with `btoa` and IE ([#&#8203;507](`axios/axios#507))
- Adding support for proxy authentication ([#&#8203;483](`axios/axios#483))
- Improving HTTP adapter to use `http` protocol by default ([#&#8203;493](`axios/axios#493))
- Fixing proxy issues ([#&#8203;491](`axios/axios#491))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.