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

test: handling failure cases properly #2206

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 32 additions & 21 deletions test/parallel/test-cluster-eaccess.js
Original file line number Diff line number Diff line change
@@ -1,41 +1,51 @@
'use strict';
// test that errors propagated from cluster children are properly
// received in their master creates an EADDRINUSE condition by also
// forking a child process to listen on a socket

var common = require('../common');
var assert = require('assert');
var cluster = require('cluster');
var fork = require('child_process').fork;
var fs = require('fs');
var net = require('net');
// Test that errors propagated from cluster workers are properly
// received in their master. Creates an EADDRINUSE condition by forking
// a process in child cluster and propagates the error to the master.

const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');
const fork = require('child_process').fork;
const fs = require('fs');
const net = require('net');

if (cluster.isMaster) {
var worker = cluster.fork();
var gotError = 0;
worker.on('message', function(err) {
gotError++;
const worker = cluster.fork();

// makes sure master is able to fork the worker
cluster.on('fork', common.mustCall(function() {}));

// makes sure the worker is ready
worker.on('online', common.mustCall(function() {}));

worker.on('message', common.mustCall(function(err) {
// disconnect first, so that we will not leave zombies
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We disconnect first, so that even if the assertion fails, the worker will not be alive.

worker.disconnect();

console.log(err);
assert.strictEqual('EADDRINUSE', err.code);
worker.disconnect();
});
}));

process.on('exit', function() {
console.log('master exited');
try {
fs.unlinkSync(common.PIPE);
} catch (e) {
}
assert.equal(gotError, 1);
});

} else {
var cp = fork(common.fixturesDir + '/listen-on-socket-and-exit.js',
{ stdio: 'inherit' });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a style error, the arguments don't line up. I have to admit I'm not a big fan of random style changes, they make the diff a lot noisier (and hence harder to read) than it needs to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it as is now.


// message from the child indicates it's ready and listening
cp.on('message', function() {
var server = net.createServer().listen(common.PIPE, function() {
console.log('parent listening, should not be!');
cp.on('message', common.mustCall(function() {
const server = net.createServer().listen(common.PIPE, function() {
// message child process so that it can exit
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the child process is alive, but the PIPE is still openable, then close the child process first and then inform the master about it.

cp.send('end');
// inform master about the unexpected situation
process.send('PIPE should have been in use.');
});

server.on('error', function(err) {
Expand All @@ -45,5 +55,6 @@ if (cluster.isMaster) {
// propagate error to parent
process.send(err);
});
});

}));
}