[diagnostics_channel] tracingChannel.traceCallback incorrect types

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.

@Qard

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.

By doc I guess that I can call like

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

but I can't 😸

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. 🤔

created #51068

doc: correct tracingChannel.traceCallback()

Navigation Menu

Repository navigation

[diagnostics_channel] `tracingChannel.traceCallback` incorrect types #50996

Affected URL(s)

Description of the problem

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

[diagnostics_channel] tracingChannel.traceCallback incorrect types #50996

Description

Affected URL(s)

Description of the problem

Activity

Flarna commented on Dec 5, 2023

Semigradsky commented on Dec 5, 2023

Qard commented on Dec 5, 2023

Flarna commented on Dec 6, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[diagnostics_channel] `tracingChannel.traceCallback` incorrect types #50996