Does not return error ENOENT when program not found on Windows

[Glavin001]

Thanks for developing cross-spawn! It is disappointing that Node.js does not support this out of the box, and your package provides something of great value to those developing packages that require support for Windows.

---

Normally, when spawn cannot find the program to be executed it returns an error ENOENT. However, since this command is wrapped in cmd.exe it no longer does that, and instead returns an error generated by cmd.exe and has returnCode=1 and stderr in the form “command” is not recognized as an internal or external command, operable program or batch file.

My workaround was to check if returnCode equals 1 (non-zero) and if the stderr message contains similar text, such as the part: is not recognized as an internal or external command.

I am using it here for my package Atom Beautify: Glavin001/atom-beautify@57ffe75

Please let me know what you think, and if I should submit a Pull Request with the workaround I described above.

[satazor]

Hi Glavin, thanks!

Yes I would be interested in working around it, but your approach won't work on non-english operating systems because the error message won't be in english. I'm not sure if there's another way to correctly identify the issue, perhaps it needs further investigation.

[Glavin001]

Bummer! Very good point. I'm not sure what we can do for non-English windows support. I would have hoped that it would return a more useful/unique error number than 1. 

I've been using node-which to help retrieve an absolute path for executables. It supports windows as well. Maybe we could use it to resolve the command executable to an absolute path and if that fails then it indicates the same as ENOENT. 

—
Sent from Mailbox

On Sun, Jun 14, 2015 at 12:34 PM, André Cruz notifications@github.com
wrote:

Hi Glavin, thanks!

Yes I would be interested in working around it, but your approach won't work on non-english operating systems because the error message won't be in english. I'm not sure if there's another way to correctly identify the issue, perhaps it needs further investigation.

Reply to this email directly or view it on GitHub:
#16 (comment)

[satazor]

Yea that would work. Perhaps what we should do is run the command and if error code is 1 we use which to disambiguate between a ENOENT or a real exit code of 1. What do you think?

[Glavin001]

I agree. How about if it it is both Windows and error code is 1 then it makes sense to do the check using which.
The error generated can be taken from here: https://github.com/joyent/node/blob/d13d7f74d794340ac5e126cfb4ce507fe0f803d5/deps/npm/lib/utils/spawn.js#L17-L21

[satazor]

Yep exactly. Can you do a PR with the implementation and tests?

[Glavin001]

For sure. I'll put it on my TODO list for over these next few days. Let me know if you have any other ideas. 

—
Sent from Mailbox

On Sun, Jun 14, 2015 at 8:10 PM, André Cruz notifications@github.com
wrote:

Yep exactly. Can you do a PR with the implementation and tests?

Reply to this email directly or view it on GitHub:
#16 (comment)

[Glavin001]

Unfortunately, it looks like spawnSync does not return ENOENT error the same as spawn does.
If I log the results from spawnSync when trying to spawn not-a-program it does not contain the field for error and so does not have ENOENT error:

{ pid: 38051,
  output: [ null, <Buffer >, <Buffer > ],
  stdout: <Buffer >,
  stderr: <Buffer >,
  status: -1,
  signal: null }
undefined '' -1
      1) should give ENOENT error for program not found

Async spawn works as expected.

Edit: Note that I am still developing on Mac, so these should be the correct error messages. I will be using AppVeyor for testing the Windows error messages.

[Glavin001]

The problem I am realizing now is that cross-spawn really handles the rewriting of the command passed into the applicable spawn method and leaves the rest up to the method. This may not really be straightforward to implement in cross-spawn without reimplementing spawn, or at least swapping its internal EventEmitter. spawn had two EventEmitters internally which allowed it to essentially watch for ENOENT error on the close event and emit a different event to the publish EventEmitter that is available when you call spawn. My workaround was using the publically available EventEmitter, however I cannot proxy the message sent to the handlers subscribing to exit or close, like Node.js does internally with spawn.

For instance, I want to detect that the spawn ended, I would listen to exit event. We need cross-spawn to also listen for this event, however when it occurs we need to also check the code and actually control the flow of the events / change the event if applicable. See https://github.com/joyent/node/blob/d13d7f74d794340ac5e126cfb4ce507fe0f803d5/deps/npm/lib/utils/spawn.js#L13-L16 for example of what Node.js's implementation of spawn does. We could have a cross-spawn EventEmitter that acts as a proxy between the spawn's EventEmitter and therefore could actually change the error messages / flow emits if applicable, in the exact same way Node.js internal spawn does. However then this does not work for spawnSync. Which we could then implement a sync-specific solution.

Just some things I want to get your thoughts on before I continue implementing.

[satazor]

What we can do is rewrite the emit function to prevent exit and close to actually emit the event and trigger the process of checking if the command exists. I can do a proof of concept in a branch and we could merge your efforts of using AppVeyor.

[satazor]

This is fixed! Landed in 1.0.0

[Glavin001]

Awesome! Thank you for implementing this. This is great! Will be testing it out on my package soon.

[satazor]

@Glavin001 Let me know if its working for you. If you are interested in how I've implemented, here it is:

https://github.com/IndigoUnited/node-cross-spawn-async/blob/master/lib/enoent.js#L14
https://github.com/IndigoUnited/node-cross-spawn/blob/master/index.js#L12