Fix Node creation crash when using plain object as options

[minggangw]

When a plain object (e.g., { enableRosout: false }) is passed as options instead of a NodeOptions instance, options.parameterOverrides is undefined, causing TypeError: Cannot read properties of undefined (reading 'length').

Added Node._normalizeOptions() to merge plain objects with NodeOptions.defaultOptions defaults using nullish coalescing (??), ensuring all documented defaults are preserved for unspecified fields while respecting explicitly set values (including false).

Files:

• lib/node.js — added static _normalizeOptions(), called at the top of _init()
• test/test-node.js — added tests for node creation with plain object options and empty object options

Fix: #1423

[Copilot]

Pull request overview

Fixes a crash in Node._init() when callers pass a plain object (or partial options object) instead of a NodeOptions instance, by ensuring parameterOverrides is treated as an empty array when missing.

Changes:

• Default options.parameterOverrides to [] before accessing .length in Node._init().
• Add regression tests covering new Node(..., { enableRosout: false }) and new Node(..., {}).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
lib/node.js	Prevents TypeError by guarding access to options.parameterOverrides.length.
test/test-node.js	Adds tests to ensure node construction does not throw with plain/partial option objects.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Wrapping the entire test body (including assertions) in assert.doesNotThrow() can obscure assertion failures as “unexpected exception”. Prefer letting Mocha fail naturally, or only wrapping the new rclnodejs.Node(...) call if the intent is specifically to assert construction doesn’t throw.

Suggested change

	assert.doesNotThrow(() => {
	const node = new rclnodejs.Node('plain_opts_node', '/ns', undefined, {
	enableRosout: false,
	});
	assert.strictEqual(node.name(), 'plain_opts_node');
	node.destroy();
	});
	});
	it('Create a node with options missing parameterOverrides', function () {
	assert.doesNotThrow(() => {
	const node = new rclnodejs.Node(
	'partial_opts_node',
	'/ns',
	undefined,
	{}
	);
	assert.strictEqual(node.name(), 'partial_opts_node');
	node.destroy();
	});
	const node = new rclnodejs.Node('plain_opts_node', '/ns', undefined, {
	enableRosout: false,
	});
	assert.strictEqual(node.name(), 'plain_opts_node');
	node.destroy();
	});
	it('Create a node with options missing parameterOverrides', function () {
	const node = new rclnodejs.Node(
	'partial_opts_node',
	'/ns',
	undefined,
	{}
	);
	assert.strictEqual(node.name(), 'partial_opts_node');
	node.destroy();

[Copilot]

Same as above: assert.doesNotThrow() around the whole callback will also treat any later assertion as an “unexpected exception”, which makes failures harder to diagnose. Consider removing it or limiting it to the constructor call.

Suggested change

	assert.doesNotThrow(() => {
	const node = new rclnodejs.Node('plain_opts_node', '/ns', undefined, {
	enableRosout: false,
	});
	assert.strictEqual(node.name(), 'plain_opts_node');
	node.destroy();
	});
	});
	it('Create a node with options missing parameterOverrides', function () {
	assert.doesNotThrow(() => {
	const node = new rclnodejs.Node(
	'partial_opts_node',
	'/ns',
	undefined,
	{}
	);
	assert.strictEqual(node.name(), 'partial_opts_node');
	node.destroy();
	});
	let node;
	assert.doesNotThrow(() => {
	node = new rclnodejs.Node('plain_opts_node', '/ns', undefined, {
	enableRosout: false,
	});
	});
	assert.strictEqual(node.name(), 'plain_opts_node');
	node.destroy();
	});
	it('Create a node with options missing parameterOverrides', function () {
	let node;
	assert.doesNotThrow(() => {
	node = new rclnodejs.Node(
	'partial_opts_node',
	'/ns',
	undefined,
	{}
	);
	});
	assert.strictEqual(node.name(), 'partial_opts_node');
	node.destroy();

[Copilot]

Since this code path is now explicitly supporting “plain object” / partial options, only defaulting parameterOverrides still leaves other NodeOptions fields as undefined (e.g. enableRosout, startParameterServices, startTypeDescriptionService), which changes behavior vs NodeOptions.defaultOptions even when callers pass {}. Consider normalizing options at the start of _init() (e.g., if it’s not a NodeOptions instance, create one with defaults and then apply provided fields via setters) so documented defaults and validation are preserved for all options, not just parameterOverrides.

[coveralls]

coverage: 85.881% (+0.05%) from 85.827%
when pulling 47116ce on minggangw:fix-1423
into 035ce9f on RobotWebTools:develop.