tracingChannel.tracePromise forces native promises

Version

18.19.0+

Platform

All

Subsystem

diagnostics_channel

What steps will reproduce the bug?

We received a report about an openai method crashing that's been wrapped with tracingChannel.tracePromise. After some digging, I see the issues is here. OpenAI creates a custom promise and it's getting stripped in tracePromise. This is a distilled repro case to show the issue.

import assert from 'node:assert';
import { tracingChannel } from 'node:diagnostics_channel';
const channel = tracingChannel('custom-promise')
channel.subscribe({
  asyncStart(data) {
  }
})

class CustomPromise {
    constructor(executor) {
        this.state = 'pending';  // Possible states: 'pending', 'fulfilled', 'rejected'
        this.value = undefined;  // Will hold the result or error
        this.successCallbacks = [];
        this.errorCallbacks = [];

        // Executor is the function passed to the promise
        try {
            executor(this._resolve, this._reject);
        } catch (error) {
            this._reject(error);
        }
    }

    // Custom resolve function
    _resolve = (value) => {
        if (this.state === 'pending') {
            this.state = 'fulfilled';
            this.value = value;
            this.successCallbacks.forEach(callback => callback(this.value));
        }
    }

    // Custom reject function
    _reject = (error) => {
        if (this.state === 'pending') {
            this.state = 'rejected';
            this.value = error;
            this.errorCallbacks.forEach(callback => callback(this.value));
        }
    }

    // Then method to handle successful promise resolution
    then(successCallback) {
        if (this.state === 'fulfilled') {
            successCallback(this.value);
        } else if (this.state === 'pending') {
            this.successCallbacks.push(successCallback);
        }
        return this;  // Allows chaining 🔄
    }

    // Catch method to handle promise rejection
    catch(errorCallback) {
        if (this.state === 'rejected') {
            errorCallback(this.value);
        } else if (this.state === 'pending') {
            this.errorCallbacks.push(errorCallback);
        }
        return this;  // Allows chaining 🔄
    }
}

function test(arg) {
  return new CustomPromise((resolve) => {
    setTimeout(() => {
      resolve(arg)
    }, 100)
  }) 
}

const arg = 'test'
const promise = channel.tracePromise(test, { ctx: true}, this, arg)
assert.equal(promise.constructor.name, 'CustomPromise')
const result = await promise
assert.equal(result, arg)

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior? Why is that the expected behavior?

To properly return the custom promise

What do you see instead?

It returns a native Promise instead

Additional information

I could workaround this by using traceSync and propagating the promise myself, but I'd prefer that the API does the right thing.

This fits the documentation which says Promise Chained from promise returned by the given function

I think for functions which are not fitting into the standard sync/callback/pattern require some hand crafted variant which fits to the lifecycle/api of the concrete object returned.

This fits the documentation which says Promise Chained from promise returned by the given function

This behaviour is currently conditional on the presence or absence of channel subscribers, and the handling of non-thenables is also variable, so there does at least need to be some formal codification of what the behaviour should be.

I think traceCallback has a similar issue. If the given callback has some extra properties set they are missing on the wrapped callback. if the called API is relying on them it likely results in problems.
Also wrappedCallback.length might differ from original.

Likely not that frequently occurring in the wild compared to thenables/custom promises.

The native promise upgrade is required for PromisePrototypeThen to work correctly.

We could eliminate the use of the primordial there. I don't recall if the primordials discussion ever came to some clear conclusion, but I know there at least was some discussion that maybe those don't matter?

I think even if we would not use PromisePrototypeThen the conversion/assumption for a Promise is needed/will happen.
A thenable is an object having a then() function. While Promise#then() returns a Promise there is no requirement for Thenable#then() to return anything.

A thenable is an object having a then() function. While Promise#then() returns a Promise there is no requirement for Thenable#then() to return anything.

A+ does mandate this, so it's not a thoroughly unreasonable assumption.

For thenables implementing the Promise spec I agree, but not for thenables which do not pretend to be a Promise.

Even our tests have thenables which don't return a Promise.

diagnostics_channel: ensure tracePromise consistency with non-Promises

diagnostics_channel: mark TracingChannel as stable

Navigation Menu

Repository navigation

tracingChannel.tracePromise forces native promises #59936

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

tracingChannel.tracePromise forces native promises #59936

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Activity

Flarna commented on Sep 21, 2025

Renegade334 commented on Sep 21, 2025

Flarna commented on Sep 22, 2025

Qard commented on Sep 25, 2025

Flarna commented on Sep 25, 2025

Renegade334 commented on Sep 25, 2025

Flarna commented on Sep 25, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions