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

test: Added relative path to accommodate limit #12601

Closed
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

test: improved call to path.relative()

Improved the call to path.relative(). Also added comments explaining the use of a relative path.
  • Loading branch information...
coreybeaumont committed Apr 27, 2017
commit b1f370b3b0966d867799fe3435de3ded85172e1c
@@ -33,8 +33,9 @@ const forAllClients = (cb) => common.mustCall(cb, CLIENT_VARIANTS);

// Test Pipe fd is wrapped correctly
{
const prefix = path.relative(`${common.PIPE}-net-connect-options-fd`,
`${__dirname}-net-connect-options-fd`);
// Using relative path on osx if the path is greaterthan 108 chars
// an AssertionError: -48 is thrown

This comment has been minimized.

Copy link
@jasnell

jasnell Apr 27, 2017

Member

nit: might be worthwhile adding a comment that explains what -48 means.

This comment has been minimized.

Copy link
@coreybeaumont

coreybeaumont Apr 27, 2017

Author Contributor

@jasnell That unfortunately I don't know. I believe @addaleax does?

This comment has been minimized.

Copy link
@Trott

Trott Apr 27, 2017

Member

I believe -48 is the libuv error for UV_EADDRINUSE. In any event, it's a libuv error.

I don't think this is OS X specific. It will happen on anything (or at least anything POSIX?) with a long enough path. I think you can just remove the comment (no one's going to wonder "why aren't they using an absolute path here??!!") or use a comment like this:

// Use relative path to avoid hitting 128-char length limit for socket paths in libuv.

/cc @addaleax to make sure I got that right (that it's 128 chars, that it's in libuv, etc.).

This comment has been minimized.

Copy link
@addaleax

addaleax Apr 27, 2017

Member

@Trott 108 should be the right number on Linux and almost certainly other systems too¹², quote unix(7):

struct sockaddr_un {
    sa_family_t sun_family;               /* AF_UNIX */
    char        sun_path[108];            /* pathname */
};

It’s this bit of code in libuv that truncates the path to that size. The actual value for EADDRINUSE is OS-dependent; for example, for me on Linux it’s 98 (which libuv forwards as -98).

See also #12708, I think that’s the same kind of problem.

¹ edit: yeah, your guess of “anything POSIX” sounds right, Windows doesn’t have UNIX sockets. ;)

² more edits: https://nodejs.org/api/net.html#net_server_listen_path_backlog_callback has numbers, it’s somewhere between 92 and 108.

const prefix = path.relative('.', `${common.PIPE}-net-connect-options-fd`);
const serverPath = `${prefix}-server`;
let counter = 0;
let socketCounter = 0;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.