@dsoprea Thanks for the PR! Could you add a new unit test where ShellJS currently would fail?

Also, what is the behavior of unix mkdir in this situation? I'd like to make sure we're matching that behavior where it makes sense to do so.

It's a difficult case to test for, for the following reasons:

1. This is a response to an anomaly that ocurs in path.dirname and only on the "win32" platform (Posix produces exactly the result that you'd expect it to: "\aa\bb" => "\aa"). So, it would require you to call path.win32.mkdir() rather than path.mkdir(). I tried setting process.platform to "win32" at the top of the process in order to trick the internal assignment logic but, apparently, the "path" module has already been loaded at that point and won't fail as expected if running on posix.
2. This only affects mkdir() when either the share is invalid (either directly, or the host is invalid, or either look invalid because of a permissions issue). The fs.existsSync() call will fail all of the way up to the host/share pair, at which point path.dirname() will start returning the same thing over and over again :) .
3. Maybe you can explain why this is happening: when I pass a bad UNC, I get a long timeout (as fs.existsSync() attempts to resolve it) followed by an uncatchable process termination.

> try { shell.mkdir('-p', '\\\\aa\\bb'); } catch(e) { console.log(e); };
ShellJS: internal error
dirname() failed: [\\aa\bb]

dustin@hostname MINGW64 /d/development/js/shelljs (master)

@dsoprea Just tried this out, and I agree it's kind of a weird scenario. Thanks for pointing me to path.win32, that let me see exactly what's happening. In response to your points:

1. This is a windows-only issue it seems, so feel free to make it a windows-only unit test.

   if (process.platform === 'win32') {
     /* whatever the test is */
   }
   

2. Sorry, you lost me at "host" and "share". However, I do see how path.dirname() can get caught in a loop returning the same thing.

3. I also observe that the shell.mkdir hangs. On node 5 win7 I see Maximum call stack size exceeded. I tried this out on Windows 7 cmd.exe and here's what I saw:

   C:\> mkdir \\aa\bb
   The network path was not found.
   

   Sounds to me like this is a valid path, it just happens to point to a non-existent network drive. Maybe that's the error we should actually be outputting? What are your thoughts?

@dsoprea any further work on this for unit tests? Be aware, #474 will also modify mkdir(), so there may be a merge conflict

A UNC has the format: "<hostname>". That's what I was referring to by "host" and "share". I'm a Unix guy, too.. I had to be reminded as well. The minimal path passed into an mkdir() call will be at least three parts: "\hostname\sharename\directory_that_does_not_exist".

I can't explain why the issue occurs inconsistently. We're just using WINS resolution (not DNS) and I can't recall whether it was a bad hostname or incorrect credentials. It also occurred in an environment that I have limited access to. I suspect that "\good_hostname\bad_sharename\folder1\folder2" would do it (either with bad_sharename being bad or incorrectly permissioned).

I think the logic is simple, succinct, and intuitive, but, as this results from a cascade-effect that only occurs when an elaborate set of conditions are met (one of which might always require a costly time delay), we're not going to be able to effectively add a test-case for this.

If you want to merge it, I'm sure we'll save someone some time. If not, then let's close it and move on.

In most of ShellJS, we don't signal errors with exceptions but by using common.error() to indicate failure information for the command (like a return code and error string like you might expect to be available from a standard shell command). @dsoprea would you be able to refactor this to follow that pattern? Otherwise, I can close this and do the refactors myself later tonight.

Done.

LGTM

Closing this in favor of #477, which includes stylistic fixes.