[diagnostics_channel] tracingChannel.traceCallback incorrect types

[Semigradsky]

Affected URL(s)

https://nodejs.org/docs/latest-v18.x/api/diagnostics_channel.html#tracingchanneltracecallbackfn-position-context-thisarg-args

Description of the problem

tracingChannel.traceCallback(fn[, position[, context[, thisArg[, ...args]]]]) - almost all args are marked as optional but it can't be called only with the first argument.

node/lib/diagnostics_channel.js

Lines 383 to 384 in 2f40652

	const callback = ArrayPrototypeAt(args, position);
	validateFunction(callback, 'callback');

^ callback is required, so we need to call it like this:

channels.traceCallback(
	function (callback) {
		// Do something
		callback(null, 'result');
	},
	undefined, // position - `-1` by default
	undefined, // context - `{}` by default
	undefined, // thisArg - any value
	callback,
)

So all arguments are required but some can be undefined.

[Flarna]

Correct. As tracingChannel.traceCallback() is built for functions receiving at least a callback as argument it's clear that user will provide at least this callback. The defaults for position, context and thisArg aren't that helpful.

For cases where callback is actually optional in the traced function traceCallback() can't be used as of now.

Seems implementation is a bit inconsistent in this regard. While a check is done that callback is actually a function it's optional inside wrappedCallback.

@Qard I think we should relax the typeof check to at least allow null/undefined. This still results in mostly useless defaults for position, context and thisArg but at least the doc is correct again.

[Semigradsky]

By doc I guess that I can call like

channels.traceCallback(
	function (callback) {
		// Do something
		callback(null, 'result');
	},
	callback,
)

but I can't 😸

[Qard]

I think I would prefer to adjust the docs to make the other args required as it may be a bit unexpected for a missing callback to be allowed when the function being wrapped potentially would have crashed if it didn't receive a callback. The traceCallback function should probably be similarly modified to only do the wrap when a callback is actually found and let the function being called fail on its own if the callback is not available.

A bit hard to decide. I'm not sure there's a clearly "correct" way to handle this generically. 🤔

[Flarna]

created #51068