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

n.kill(19) throws error #9519

Closed
ORESoftware opened this issue Nov 8, 2016 · 11 comments
Closed

n.kill(19) throws error #9519

ORESoftware opened this issue Nov 8, 2016 · 11 comments
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.

Comments

@ORESoftware
Copy link
Contributor

ORESoftware commented Nov 8, 2016

On versions 6 and 7 of Node.js

when I started a child process with const n = require('child_process').spawn

if I try,

n.kill(19), I get an error saying the signal is not recognized

but 19 is a valid signal
http://stackoverflow.com/questions/9951556/why-number-9-in-kill-9-command-in-unix

I would expect it to work

if I do n.kill('SIGSTOP'), it seems to work.

Can we (not) use numbers/integers with n.kill()?

@mscdex mscdex added question Issues that look for answers. child_process Issues and PRs related to the child_process subsystem. labels Nov 8, 2016
@Fishrock123
Copy link
Contributor

Can we (not) use numbers/integers with n.kill()?

That is my (perhaps poor) understanding.

@cjihrig
Copy link
Contributor

cjihrig commented Nov 9, 2016

Again, can you please ask these general support questions at https://github.com/nodejs/help.

@cjihrig cjihrig closed this as completed Nov 9, 2016
@sam-github sam-github added process Issues and PRs related to the process subsystem. doc Issues and PRs related to the documentations. confirmed-bug Issues with confirmed bugs. and removed question Issues that look for answers. labels Nov 9, 2016
@sam-github sam-github reopened this Nov 9, 2016
@sam-github
Copy link
Contributor

This is a doc bug (besides being an implied reasonable feature request):

@sam-github sam-github added the good first issue Issues that are suitable for first-time contributors. label Nov 9, 2016
@sam-github
Copy link
Contributor

@ORESoftware use strings for now, or use process._kill() if you are desperate, its not an official API, though, and may disappear at any time.

@ORESoftware
Copy link
Contributor Author

@cjihrig yeah np, this seemed like a more serious one though. Seems like it would be very easy to support numbers as well as strings, very unexpected.

@cjihrig
Copy link
Contributor

cjihrig commented Nov 9, 2016

@ORESoftware apologies. It looks like you had a valid bug report.

@Fishrock123 Fishrock123 removed the confirmed-bug Issues with confirmed bugs. label Nov 9, 2016
@sam-github
Copy link
Contributor

@ORESoftware any interest in PRing numeric signal support? And/or the doc fixes? I'll be able to review or merge, but I won't have time to implement this week, someone else may pick them up, but if you have time and interest in contributing to the project, these are good first contributions.

@ORESoftware
Copy link
Contributor Author

Yeah I could give that a shot

On Nov 9, 2016 11:06 AM, "Sam Roberts" notifications@github.com wrote:

@ORESoftware https://github.com/ORESoftware any interest in PRing
numeric signal support? And/or the doc fixes? I'll be able to review or
merge, but I won't have time to implement this week, someone else may pick
them up, but if you have time and interest in contributing to the project,
these are good first contributions.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9519 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AKn56O-GR_5bHi6UZ9nk5GggTp3856amks5q8hmogaJpZM4Ks5Tp
.

@gerrard00
Copy link
Contributor

I don't mean to step on any toes, but I took a cut at this. I've been looking for something to get my feet wet and this seemed easy enough. Your feedback is greatly appreciated.

#9651

gerrard00 added a commit to gerrard00/node that referenced this issue Nov 22, 2016
@Trott
Copy link
Member

Trott commented Jul 15, 2017

@sam-github Should this stay open?

@richardlau
Copy link
Member

I think this is fixed by #10423

@gibfahn gibfahn closed this as completed Jul 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants