Mutating traced function arguments in channel subscriber

[simon-id]

Thank you @sabrenner for discovering, reporting, and helping with this!

Sometimes we want to replace one of the arguments of the traced function, from inside of the channel subscriber. This is usually the case when we want to replace the callback by our own, to be able to execute code before the real callback is called:

function fetch (data, cb) {
  cb(data)
}

This gets rewritten into:

function fetch(data, cb) {
    const traced = ()=>{
        cb(data);
    };
    if (!tr_ch_apm$param_mutation.hasSubscribers) return traced();
    return tr_ch_apm$param_mutation.traceSync(traced, {
        arguments,
        self: this
    });
}

and we can then run:

channel.subscribe((context) => {
  const originalCb = context.arguments[1]
  const wrappedCb = function (data) {
    // ... do something before real callback is called
    return originalCb.apply(this, arguments)
  }

  context.arguments[1] = wrappedCb
})

This already work because the named arguments of fetch() are updated when the arguments object is mutated.

But because of ECMA-262 specification 10.2.11 22.a.i, it does NOT work in strict-mode functions, or in functions with "complex" arguments (ie: rest parameters, default values, destructured parameters):

22. If argumentsObjectNeeded is true, then
    a. If strict is true or simpleParameterList is false, then
       i. Let ao be CreateUnmappedArgumentsObject(argumentsList).
    b. Else,
       i. NOTE: A mapped argument object is only provided for non-strict functions that don't have a rest parameter, any parameter default value initializers, or any destructured parameters.
       ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).

So if we replace the initial code with something as trivial as:

function fetch ({ data }, cb) { // same thing with `...data` or `data = 'default'` or with 'use strict'
  cb(data)
}

the subscriber cannot replace the callback anymore.

[simon-id]

It is not possible to fix this by cloning the original arguments in the traced() function so they overshadow the top level arguments, like so:

function fetch(data, cb) {
    const traced = (data,cb)=>{
        cb(data);
    };
    if (!tr_ch_apm$param_mutation.hasSubscribers) return traced();
    return tr_ch_apm$param_mutation.traceSync(traced, {
        arguments,
        self: this
    }, this, ...arguments);
}

because traceSync() and the diagnostics_channel internals all use spread operators to pass args to the original function, which breaks the link with the original arguments object, and thus prevents mutation:
nodejs/lib/diagnostics_channel.js#L320

  traceSync(fn, context = {}, thisArg, ...args) {
    if (!this.hasSubscribers) {
      return ReflectApply(fn, thisArg, args);
    }

    const { start, end, error } = this;

    return start.runStores(context, () => {
      try {
        const result = ReflectApply(fn, thisArg, args);
        context.result = result;
        return result;
      } catch (err) {
      ...