spawn error doesn't utilize event handlers

Nodejs version 6.10.x

I have this:

const file = path.resolve(__dirname + '/foo.js');
const  n = cp.spawn(file, []);  // EACCES error is thrown here

n.once('error', function () {
  // this does not get hit !!
});

so it seems like I should do this instead (but the try/catch seems unfortunate):

let n;

try{
  n = cp.spawn(file, []);  //  EACCES error is thrown here
}
catch(err){
  return cb(err);
}

n.once('error', function () {
  // this does not get hit !!
});

seems like the 'error' handler should be given the chance to capture error, instead of necessitating a try/catch, am I doing something wrong?

Perhaps because Node core does not even create the n child process object, that seems like an implementation "mistake".

HOW TO REPLICATE:

Create a file without executable permissions, then try to execute it directly:

#!/usr/bin/env bash
touch my-non-executable-file.js
./my-non-executable-file.js

so that would be something like:

const k = cp.spawn('/foo/bar/my-non-executable-file.js');

the above should throw an error, instead of using k.on('error', fn)

one can argue that n represents a child process and in this case as the process was never created, n not being fully formed makes sense.

Technically it may be possible to first compose a JS object that is going to abstract the child process that is being spawned. But a more natural way of doing this is to materialize the entity first, and then abstract the entity. (the entity here being the native process).

Is it against your convictions?

@bnoordhuis

eh, because the child process was never "started", I can't use an 'exit' handler on the child process, which forces me to rethink my own program design. It seems like a child was started in reality, but I don't know the internals that well. I don't see why you'd ever have to put a try/catch around spawn. IMO a better design would be to always use n.on('error').

@bnoordhuis this seems like a design mistake in node core. Having to put try/catch around

try{
cp.spawn('node')
}
catch(err){
}

seems wrong :) I hate try/catch lol

this block of code defines what is runtime error and what is a linkage error, and the current list does not contain UV_EACCES. If I add this patch the above code runs fine:

--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@@ -26,7 +26,8 @@ const {
   UV_ENFILE,
   UV_ENOENT,
   UV_ENOSYS,
-  UV_ESRCH
+  UV_ESRCH,
+  UV_EACCES
 } = process.binding('uv');
 
 const errnoException = errors.errnoException;
@@ -308,6 +309,7 @@ ChildProcess.prototype.spawn = function(options) {
   if (err === UV_EAGAIN ||
       err === UV_EMFILE ||
       err === UV_ENFILE ||
+      err === UV_EACCES ||
       err === UV_ENOENT) {
     process.nextTick(onErrorNT, this, err);
     // There is no point in continuing when we've hit EMFILE or ENFILE

question is on the protocol definition of runtime error and whether UV_EACCES comes under its purview or not.

/cc @nodejs/child_process

child_process: define EACCES as a runtime error

Navigation Menu

Repository navigation

spawn error doesn't utilize event handlers #990

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

spawn error doesn't utilize event handlers #990

Description

Activity

gireeshpunathil commented on Nov 25, 2017

ORESoftware commented on Nov 25, 2017

gireeshpunathil commented on Mar 7, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions