Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add utility to synchronously inspect promise state #40054

Closed
sindresorhus opened this issue Sep 9, 2021 · 26 comments
Closed

Add utility to synchronously inspect promise state #40054

sindresorhus opened this issue Sep 9, 2021 · 26 comments
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@sindresorhus
Copy link

Is your feature request related to a problem? Please describe.

In most cases, you should just await the promise and get its value, even if the promise has already resolved. However, it could sometimes be useful to get the state of the promise. For example, as an optimization to only do a heavy operation if the promise in a certain state or for assertions when writing tests.

Describe the solution you'd like

There used to be process.binding('util').getPromiseDetails(promise), but it was removed in Node.js 16. I suggest exposing a util.promiseState(promise) method which would return 'pending' | 'fulfilled' | 'rejected' depending on the promise state.

Describe alternatives you've considered

I have made a package for this, but it depends on parsing util.inspect() output which is a bit fragile.

@devsnek
Copy link
Member

devsnek commented Sep 9, 2021

I am strongly opposed to exposing this functionality.

@sindresorhus
Copy link
Author

sindresorhus commented Sep 9, 2021

@devsnek Why? Reflection capabilities are quite common in other languages.

@devsnek
Copy link
Member

devsnek commented Sep 9, 2021

@sindresorhus we have reflection capabilities in node as well, check out the inspector module. Anything more first-class would be improper to expose as it would encourage people to do weird zalgo timing "optimizations" or have strange zalgo test assertions.

@sindresorhus
Copy link
Author

Anything can be misused. Good docs can help prevent that. Node.js has lots of synchronous APIs, for example, even though it can be misused in a server setting.

@jasnell
Copy link
Member

jasnell commented Sep 9, 2021

@sindresorhus ... What's the basic use case you have in mind? Specifically, why do you want to know the pending status vs. just getting the results?

@Mesteery Mesteery added the feature request Issues that request new features to be added to Node.js. label Sep 9, 2021
@szmarczak
Copy link
Member

@jasnell One use case would be test assertions.

@devsnek
Copy link
Member

devsnek commented Sep 9, 2021

why would you need this for a test assertion?

@voxpelli
Copy link

voxpelli commented Sep 9, 2021

Can't this mostly be solved with a wrapper roughly like this:

const getPromiseWithStatus = (input) => {
  const result = Promise.resolve(input);
  result.status = 'pending';
  result.then(
    () => result.status = 'fulfilled',
    () => result.status = 'rejected'
  );
  return result;
};

Sure, if given an already fulfilled/rejected promise it will not be able to give an answer in sync, but if wrapped early on it can?

@Jamesernator
Copy link

For the assertions thing I've used basically the same as the approach above and have had no issues.

For optimization cases I doubt there's many uses that need to get the resolved value of a promise from an arbitrary source. Most cases that are happy to commit to supporting having a value available earlier will provide a synchronous version or an event.

I can see uses for getting the resolved value of promises vended from the same code that needs to access it, but this should be doable with some simple helpers.

@silverwind
Copy link
Contributor

silverwind commented Sep 10, 2021

I feel this is better solved on language level, e.g. raise an ECMAScript proposal. Having a non-standard node API would be detrimental.

@Richienb
Copy link
Contributor

Just discovered this sweet solution:

function promiseStateSync(promise) {
	let state = 'pending';
	
	promise
		.then(() => {
			state = 'fulfilled';
		})
		.catch(() => {
			state = 'rejected';
		});

	process._tickCallback();

	return state;
}

The only thing I'm uncertain of is whether process._tickCallback() will be removed in the future since it's been listed as pending deprecation for quite some time now. It would be nice if someone could finish up #29671.

@sindresorhus
Copy link
Author

What's the basic use case you have in mind? Specifically, why do you want to know the pending status vs. just getting the results?

And it's quite a popular need: https://stackoverflow.com/questions/30564053/how-can-i-synchronously-determine-a-javascript-promises-state

@sindresorhus
Copy link
Author

Can't this mostly be solved with a wrapper roughly like this:

@voxpelli That's pretty much how my package works: https://github.com/sindresorhus/p-state/blob/main/browser.js

@sindresorhus
Copy link
Author

I feel this is better solved on language level, e.g. raise an ECMAScript proposal. Having a non-standard node API would be detrimental.

@silverwind You could use that argument about a lot of the existing Node.js APIs. Having it in the language would be ideal, but it's a much higher barrier.

@jasnell
Copy link
Member

jasnell commented Sep 11, 2021

The other challenge with having it at the language level is that this has been considered there already as far as I'm aware. I'm not opposed to us exposing an API, I'm just concerned about it being misused. @sindresorhus' use cases are legitimate but it's entirely likely that folks will come up with more problematic uses.

Another challenge here, however, is that whatever we might add here will be specific to Node.js and won't work in other platforms (browsers, deno, workers, etc). I think we can expose the utility, with heavy warnings in the docs, then see if tc39 is willing to reconsider.

@devsnek
Copy link
Member

devsnek commented Sep 11, 2021

I can just say now, the likelihood of tc39 finding consensus on such a feature is basically zero. the entire point of promises is that you can't inspect their state synchronously.

for your optimization use case, I would just suggest not using promises. if the code is a public api and you can't not use promises that's a good sign that your optimization would be a brittle zalgo issue.

for testing, this test makes no sense to me. why does this test care about the tick by tick state of the promise? I'd just set a done variable synchronously after writing the rest of the body and assert done is true in a .then() handler on the promise (you could also assert the innerhtml value matches a full expected result instead of having a separate boolean). that asserts a correct ordering without ever synchronously testing the state of the promise.

@addaleax
Copy link
Member

At the risk of spilling some “forbidden” knowledge:

$ cat > test.js <<EOF
const vm = require('vm');
const p = Promise.resolve(42);

vm.runInNewContext(
  "p.then(() => p.status = 'fulfilled', () => p.status = 'rejected');", 
  { p }, 
  { microtaskMode: 'afterEvaluate' })

console.log(p.status);
EOF
$ node -v
v14.17.6
$ node test.js
fulfilled

@Richienb
Copy link
Contributor

🤯

Version without mutations:

import vm from 'node:vm';

export function promiseStateSync(promise) {
	const data = {
		state: 'pending',
		promise,
	};

	vm.runInNewContext(`
		data.promise
			.then(() => {
				data.state = 'fulfilled';
			})
			.catch(() => {
				data.state = 'rejected';
			})
	`, {data}, {microtaskMode: 'afterEvaluate'});

	return data.state;
}

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Apr 4, 2022
@szmarczak
Copy link
Member

unstale

@github-actions github-actions bot removed the stale label Apr 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Oct 2, 2022
@Richienb
Copy link
Contributor

Richienb commented Oct 2, 2022

unstale

@github-actions github-actions bot removed the stale label Oct 3, 2022
@jlee474
Copy link

jlee474 commented Jan 2, 2023

@sindresorhus ... What's the basic use case you have in mind? Specifically, why do you want to know the pending status vs. just getting the results?

Use case would be in Promise.race. for instance,
Promise.race([promise1, promise2, promise3]), and you need to know which Promise won the race.
So if promise2 won, then promise1 and promise3 would be in pending status.

@voxpelli
Copy link

voxpelli commented Jan 2, 2023

Use case would be in Promise.race. for instance, Promise.race([promise1, promise2, promise3]), and you need to know which Promise won the race. So if promise2 won, then promise1 and promise3 would be in pending status.

@jlee474 That's something you can use Promise.race itself for, its eg. documented in MDN and MDN mentions promise-status-async as an implementation if it.

Core principle:

const nonPendingValue = {};

if (nonPendingValue === await Promise.race([possiblyPendingPromise, nonPendingValue])) {
  // possiblyPendingPromise is still pending
}

I think you would need to define when there would be a need to do it synchronously.

@bnoordhuis
Copy link
Member

Various solutions have been mentioned and there's no consensus about adding this to node natively so I'm going to go ahead and close this out.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2023
@jcuffe
Copy link

jcuffe commented Mar 13, 2024

Found this while looking for an idiomatic solution for inspecting promises. Our use case is a NodeJS app with a few hundred tests, each of which may or may not create a promise which "outlives" the test logic, introducing race conditions and flaky test runs. We want to identify these tests without needing to read each one and trace every function call. We can use a mocha plugin:

let OriginalPromise;
const trackedPromises = new Set();

module.exports = {
  mochaHooks: {
    beforeEach() {
      OriginalPromise = Promise;
      class TrackedPromise extends OriginalPromise {
        constructor(...args) {
          const promise = super(...args);
          trackedPromises.add(promise);

          const untrackPromise = () => trackedPromises.delete(promise)
          promise.then(untrackPromise, untrackPromise)

          return promise;
        }

        // Prevent infinite recursion when using superclass constructor
        static get [Symbol.species]() {
          return OriginalPromise;
        }
      }

      Promise = TrackedPromise;
    },

    afterEach() {
      Promise = OriginalPromise;

      if (trackedPromises.size) {
        console.log('Unsettled promise after test completion', {
          suiteName: this.currentTest.parent?.title,
          testName: this.currentTest.title,
        });

        trackedPromises.clear();
      }
    },
  },
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

16 participants