process: throw TypeError if kill pid not a number

[sam-github]

Fix #7962

Note that this PR disallows string forms of process ids, so will break code like:

// kill.js
process.kill(process.argv[2])

called as node kill.js 6543.

I could allow string forms of numbers, but disallow strings that are NOT numbers, I could also allow both null and undefined to convert to 0, meaning self, but that seems weird. Note that +null is 0 in js, and +undefined is NaN, though how much sense that makes, I'm not sure.

Node, in general, does precious little argument checking, its hard to know what would be expected (other than calling APIs in contradiction of their docs is not going to work well).

[cjihrig]

If you're going to verify that pid is a number, then it might be worth adding an isFinite() check to rule out Infinity and NaN.

[trevnorris]

Alternatively you can do a type coercion if the string can be properly coerced to a string. For example:

if (typeof pid !== 'number' || !isNaN(pid)) {
  // This forces a uint coercion. So might also want to check
  // if the pid is > 0.
  pid = pid >>> 0;

[trevnorris]

Note: Updated the first example for syntax error, and && should have been ||.

[sam-github]

"if the string can be properly coerced to a string", did you mean to a number?

I explicitly disallow string forms of numbers, if you grep 'TypeError.*number' lib/*.js, I think you'll find that (mostly) util.isNumber() is used for similar checks, and it doesn't allow string forms of numbers.

It also passes for NaN and Infinity.... but that's another story :-)

[bnoordhuis]

Note that this PR disallows string forms of process ids

That doesn't seem like an appropriate change for the stable branch.

I think that the check should be pid === (pid | 0) because pid === (pid >>> 0) is false for negative values. pid_t is a signed integral type and kill(-1) has well-defined semantics, QED.

EDIT: Replace === with == if type coercion should be allowed - which arguably it should.

[indutny]

I totally agree with @bnoordhuis , let's do it in v0.11

[indutny]

Landed in 832ec1c, thank you!

[trevnorris]

@bnoordhuis good point about being able to pass -1. Well then, there are three syntax choices to pass :-P

pid === (~~pid);
pid === (pid >> 0);
pid === (pid | 0);