doc: document the return value of process.send() and worker.send() #27003

shree-y · 2019-03-30T02:28:24Z

Checklist

documentation is changed or added
commit message follows commit guidelines

sam-github · 2019-03-30T03:23:26Z

doc/api/cluster.md

@@ -459,6 +459,11 @@ if (cluster.isMaster) {
 }
 ```

+`worker.send()` will return `false` if the channel has closed or when the


I think it will throw if the channel is closed, could you try?

Okay, I am not sure how to try. I will see if there is a unit test for this.

I found this test: test-child-process-fork-closed-channel-segfault.js. It does look like it throws an error, if the channel is closed. I will update the doc.

sam-github · 2019-03-30T03:25:06Z

doc/api/cluster.md

@@ -459,6 +459,11 @@ if (cluster.isMaster) {
 }
 ```

+`worker.send()` will return `false` if the channel has closed or when the
+backlog of unsent messages exceeds a threshold that makes it unwise to send
+more. Otherwise, the method returns `true`. The `callback` function can be


If you are going to describe the callback function, probably best to say when it is called (after the message has been sent), then describe what it could be used for.

shree-y · 2019-04-11T20:29:30Z

@sam-github Hi, I addressed your feedback and pushed it up. Just wondering if there is anything else you would like me to do. Thanks!

doc/api/process.md

lpinca · 2019-04-24T17:58:54Z

Ping @nodejs/documentation @nodejs/child_process

lpinca · 2019-04-25T19:07:08Z

@shree-y the linter is not happy, see https://travis-ci.com/nodejs/node/jobs/195352578. Can you please fix that?

See also https://travis-ci.com/nodejs/node/jobs/195352579

shree-y · 2019-04-25T20:11:39Z

@shree-y the linter is not happy, see https://travis-ci.com/nodejs/node/jobs/195352578. Can you please fix that?

See also https://travis-ci.com/nodejs/node/jobs/195352579

Sorry about that. I will fix it.

shree-y · 2019-04-26T03:07:34Z

@lpinca I fixed one linter issue.

Not sure how to fix this: https://travis-ci.com/nodejs/node/jobs/195767038. Can you please help?

lpinca · 2019-04-26T05:39:10Z

@shree-y you should rebase, amend the commit message title and body, and then force push. This should work for the first commit:

doc: document the return value of {process,worker}.send()

Add documentation for the boolean value returned by process.send()
and worker.send().

Fixes: https://github.com/nodejs/node/issues/26995

The same rules apply to all commits, unless they are only there to facilitate review and need to be squashed before landing.

Let me know if you need guidance on how to use git to rebase, amend and force push.

shree-y · 2019-04-26T22:48:43Z

@lpinca Thanks, finally figured it out!

doc/api/process.md

lpinca

SGTM.

shree-y · 2019-04-29T23:01:07Z

@lpinca Is there anything else I need to do to have this PR merged?

lpinca · 2019-04-30T04:55:10Z

We need another approval and no objections / requests for changes.
@sam-github PTAL.

doc/api/cluster.md

Add documentation for the boolean value returned by process.send() and worker.send(). Fixes: nodejs#26995

sam-github

sorry, the docs still aren't accurate. Probably best to just add links to the docs that describe the behaviour already, in detail (ChildProcess.send).

sam-github · 2019-06-07T17:51:34Z

doc/api/cluster.md

+
+
+`worker.send()` will throw an error if the channel has closed or when the
+backlog of unsent messages exceeds a threshold that makes it unwise to send


This isn't correct. For master, worker.send basically goes through a couple layers, and ends up being a direct call of https://nodejs.org/api/child_process.html#child_process_subprocess_send_message_sendhandle_options_callback. I suggest redirecting to those docs from here.

I'd have to go spelunking through the code, but I strongly suspect that in child processes, .send() is same as process.send(), and that ends up at an internal function that also has the same interface as the ChildProcess.send(). forking a child and doing console.log(process.send) might be easier than reading the code.

That .send throws when unsent messages exceeds a backlog is definitely not the case, unless I very, very much misread the code.

gireeshpunathil · 2019-12-18T12:48:29Z

@sam-github - what is the next step here? could you pls guide @shree-y on the modification to the doc, or suggest an alternative?

sam-github · 2019-12-18T16:05:04Z

I have two unresolved comments, both of them have specific suggestions for change.

gireeshpunathil · 2019-12-22T11:53:04Z

ping @shree-y - hope you are able to address @sam-github 's review comments?

HarshithaKP · 2020-03-31T07:18:48Z

@shree-y, will be able to address review comments? and this needs a rebase.

jasnell · 2020-06-25T01:21:15Z

This has not been worked on in a while and is out of date. Closing but can reopen if it is picked back up and updated

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 30, 2019

shree-y changed the title ~~doc: Update with the missing documentation of the meaning of send() or process.send() and worker.send()~~ doc: Update the docs with the missing documentation for the meaning of send() for process.send() and worker.send() Mar 30, 2019

shree-y changed the title ~~doc: Update the docs with the missing documentation for the meaning of send() for process.send() and worker.send()~~ doc: Update the missing documentation with the meaning of the boolean return value of send() for process.send() and worker.send() Mar 30, 2019

sam-github suggested changes Mar 30, 2019

View reviewed changes

shree-y force-pushed the update-api-doc-to-describe-send-fn-return-meaning branch from 828fa29 to c1e02f7 Compare April 2, 2019 05:35

BridgeAR changed the title ~~doc: Update the missing documentation with the meaning of the boolean return value of send() for process.send() and worker.send()~~ doc: document the return value of process.send() and worker.send() Apr 16, 2019

shree-y mentioned this pull request Apr 24, 2019

meaning of the boolean return value of send() not documented #26995

Closed

lpinca reviewed Apr 24, 2019

View reviewed changes

doc/api/process.md Show resolved Hide resolved

lpinca added the cluster Issues and PRs related to the cluster subsystem. label Apr 24, 2019

shree-y closed this Apr 25, 2019

shree-y reopened this Apr 25, 2019

shree-y force-pushed the update-api-doc-to-describe-send-fn-return-meaning branch 2 times, most recently from 978b711 to 88d4b07 Compare April 26, 2019 02:31

shree-y force-pushed the update-api-doc-to-describe-send-fn-return-meaning branch 3 times, most recently from 2463848 to 2edc84a Compare April 26, 2019 22:31

lpinca reviewed Apr 27, 2019

View reviewed changes

doc/api/process.md Show resolved Hide resolved

lpinca approved these changes Apr 27, 2019

View reviewed changes

oyyd reviewed May 8, 2019

View reviewed changes

doc/api/cluster.md Outdated Show resolved Hide resolved

shree-y added 2 commits May 23, 2019 21:22

doc: document the return value of {process,worker}.send()

782e702

Add documentation for the boolean value returned by process.send() and worker.send(). Fixes: nodejs#26995

doc: document the return value of {process,worker}.send()

9426474

Add documentation for the boolean value returned by process.send() and worker.send(). Fixes: nodejs#26995

shree-y added 2 commits May 23, 2019 21:22

doc: document the return value of {process,worker}.send()

6fccf84

Add documentation for the boolean value returned by process.send() and worker.send(). Fixes: nodejs#26995

Re-arranged Worker and Error in alphabetical order

9101de7

shree-y force-pushed the update-api-doc-to-describe-send-fn-return-meaning branch from 88720cc to 9101de7 Compare May 24, 2019 03:23

sam-github suggested changes Jun 7, 2019

View reviewed changes

Trott force-pushed the master branch from 1ecc406 to 49cf67e Compare September 17, 2019 16:51

devnexen force-pushed the master branch from e8a4568 to 5289f80 Compare December 26, 2019 19:46

BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:19

jasnell added the stalled Issues and PRs that are stalled. label Jun 19, 2020

jasnell closed this Jun 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: document the return value of process.send() and worker.send() #27003

doc: document the return value of process.send() and worker.send() #27003

shree-y commented Mar 30, 2019

sam-github Mar 30, 2019

shree-y Apr 1, 2019

shree-y Apr 2, 2019

sam-github Mar 30, 2019

shree-y commented Apr 11, 2019

lpinca commented Apr 24, 2019 •

edited

Loading

lpinca commented Apr 25, 2019

shree-y commented Apr 25, 2019

shree-y commented Apr 26, 2019

lpinca commented Apr 26, 2019

shree-y commented Apr 26, 2019

lpinca left a comment

shree-y commented Apr 29, 2019

lpinca commented Apr 30, 2019

sam-github left a comment

sam-github Jun 7, 2019

gireeshpunathil commented Dec 18, 2019

sam-github commented Dec 18, 2019

gireeshpunathil commented Dec 22, 2019

HarshithaKP commented Mar 31, 2020

jasnell commented Jun 25, 2020



		`worker.send()` will throw an error if the channel has closed or when the
		backlog of unsent messages exceeds a threshold that makes it unwise to send

doc: document the return value of process.send() and worker.send() #27003

doc: document the return value of process.send() and worker.send() #27003

Conversation

shree-y commented Mar 30, 2019

Checklist

sam-github Mar 30, 2019

Choose a reason for hiding this comment

shree-y Apr 1, 2019

Choose a reason for hiding this comment

shree-y Apr 2, 2019

Choose a reason for hiding this comment

sam-github Mar 30, 2019

Choose a reason for hiding this comment

shree-y commented Apr 11, 2019

lpinca commented Apr 24, 2019 • edited Loading

lpinca commented Apr 25, 2019

shree-y commented Apr 25, 2019

shree-y commented Apr 26, 2019

lpinca commented Apr 26, 2019

shree-y commented Apr 26, 2019

lpinca left a comment

Choose a reason for hiding this comment

shree-y commented Apr 29, 2019

lpinca commented Apr 30, 2019

sam-github left a comment

Choose a reason for hiding this comment

sam-github Jun 7, 2019

Choose a reason for hiding this comment

gireeshpunathil commented Dec 18, 2019

sam-github commented Dec 18, 2019

gireeshpunathil commented Dec 22, 2019

HarshithaKP commented Mar 31, 2020

jasnell commented Jun 25, 2020

lpinca commented Apr 24, 2019 •

edited

Loading