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

cluster.on('message') not return worker #5764

Closed
Hamper opened this issue Mar 17, 2016 · 13 comments
Closed

cluster.on('message') not return worker #5764

Hamper opened this issue Mar 17, 2016 · 13 comments
Assignees
Labels
cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations.

Comments

@Hamper
Copy link

Hamper commented Mar 17, 2016

  • Version: 4.4.0, 5.9.0
  • Platform: Linux test 4.4.4-calculate deps: update openssl to 1.0.1j #1 SMP PREEMPT Wed Mar 9 16:35:58 MSK 2016 x86_64 Intel(R) Core(TM) i5-3470 CPU @ 3.20GHz GenuineIntel GNU/Linux
  • Subsystem: cluster, documentation

In documentation:

Event: 'message'#
worker <cluster.Worker>
message <Object>
Emitted when any worker receives a message.

but real use is:

cluster.on('message', msg => { /* worker isn`t known */ })
@Fishrock123 Fishrock123 added the cluster Issues and PRs related to the cluster subsystem. label Mar 17, 2016
@Fishrock123
Copy link
Contributor

@Hamper Try: cluster.on('message', (msg, worker) => { })?

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Mar 17, 2016
@Fishrock123
Copy link
Contributor

It does looks like order of the arguments as told by the docs is incorrect though.

@Hamper
Copy link
Author

Hamper commented Mar 17, 2016

'use strict'
const cluster = require('cluster')

if (cluster.isMaster) {
    cluster.on('message', (msg, wrk) => console.log(`msg=${msg}, wrk=${wrk}`))
    cluster.fork()
} else {
    cluster.worker.send('test')
    process.send('test2')
}

result:

msg=test, wrk=undefined
msg=test2, wrk=undefined

@mscdex
Copy link
Contributor

mscdex commented Mar 17, 2016

The documentation agrees with master, but not 5.x/4.x.

@MylesBorins
Copy link
Contributor

a doc fix PR should be made targeting both v5.x and v4.x-staging

edit: this will need to be 2 pr's

@mscdex
Copy link
Contributor

mscdex commented Mar 17, 2016

Looks like the worker argument was added/fixed in 66f4586.

@jasnell
Copy link
Member

jasnell commented May 1, 2016

@nodejs/documentation

@Cobraeti
Copy link

Hi,
Is it planed to be fixed in LTS version?
I just downloaded the last one (v4.4.4 from 05/05/2016) and it is still behaving as discribed in @Hamper 's first post...

@MylesBorins MylesBorins self-assigned this May 17, 2016
@Knighton910
Copy link

In the top of your else statement try this process.on('message')

@Cobraeti
Copy link

Cobraeti commented May 18, 2016

Hi @lordKnighton ,
I already use process.on('message') as my worker emits that event in a sub-sub-sub-module and I don't want to pass the Cluster object to all intermediate module ^^
The problem is in the other way, when using the process.send() method (worker's syde).
As described by @Hamper , the cluster.on('message') method receives only a message parameter, so when using the following code:

var cluster = require('cluster');

if(cluster.isMaster) {
    cluster.on('message', function(worker, message) {
        console.log(`Worker ${worker.id} said "${message}"`);
        console.log(`Worker ${worker} said "${message}"`);
    }
    cluster.fork();
}
else {
    process.send('Hello World!!!!');
}

The result in console is:

Worker unknow said "unknow"
Worker Hello World!!!! said "unknow"

@Cobraeti
Copy link

I've just tested the same code within Node.js v6.1.0 and it works like a charm.
It seems that the change has been implemented in that version (don't know for v5.x).
Hopefully, my dependencies can now compile again (that was not the case whith v5.x a few weeks ago), so I would definitely switch to this better one.
I hope my comment will at least help to bring up the issue for those who can't switch version...

cjihrig added a commit to cjihrig/node that referenced this issue Jun 14, 2016
This commit corrects the cluster message event signature.

Fixes: nodejs#5764
@cjihrig
Copy link
Contributor

cjihrig commented Jun 14, 2016

A fix for v4.x-staging is at #7297. I'm inclined to not fix v5.x, as it is EOL in a couple weeks.

cjihrig added a commit that referenced this issue Jun 15, 2016
This commit corrects the cluster message event signature.

Fixes: #5764
PR-URL: #7297
Reviewed-By: Brian White <mscdex@mscdex.net>
@cjihrig
Copy link
Contributor

cjihrig commented Jun 15, 2016

Closed in 47f82cd.

@cjihrig cjihrig closed this as completed Jun 15, 2016
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
This commit corrects the cluster message event signature.

Fixes: #5764
PR-URL: #7297
Reviewed-By: Brian White <mscdex@mscdex.net>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
This commit corrects the cluster message event signature.

Fixes: #5764
PR-URL: #7297
Reviewed-By: Brian White <mscdex@mscdex.net>
@sam-github sam-github added doc Issues and PRs related to the documentations. and removed doc Issues and PRs related to the documentations. labels Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

9 participants