Why am I getting an unexpected behavior in my implementation of Promise.race()?

Question

I have the code for my Promise.race() implementation, and it basically works. However, in this case:

promiseRace([Promise.reject('test_error'), Promise.resolve('test_value')])

I am getting test_value instead of test_error.
Why it happens?

const promiseRace = promises => new Promise((resolve, reject) => {
  promises.forEach(p => {
    if (
      typeof p === 'object'
          && 'then' in p
          && typeof p.then === 'function'
    ) {
      p.then(resolve).catch(reject);
    } else {
      resolve(p);
    }
  });
});

promiseRace([Promise.reject('test_error'),
  Promise.resolve('test_value')]).then(value => { console.log(value); });

Can you explain why you think the result should be `test_error`? I know `Promise.race` results in `test_error`, but why do you think _your_ code should do this too? Walk us through your reasoning behind what each line in your function is meant to do, so that folks can point out where your assumptions don't match with how JS has been designed to work. Also, note that you don't need that big if statement, `if (p instanceof Promise)` is all you need. Also note you're not concerned with preserving "`this`", so writing this as a normal `function promiseRace(promises) { ... }` would be fine. — Mike 'Pomax' Kamermans, Oct 16 '21 at 16:33
This option is less beautiful, however, if we are talking about two tabs (yes, in this case instanceof is justified), then there will be problems with instanceof, because if you pass code from one tab to another - there are different Promise objects — BERTOLDO, Oct 16 '21 at 16:54
That doesn't matter? If it's a promise, `p instanceof Promise` is true, the end. Whether you pass code around or not makes no difference: anything that's a promise, is a promise, and thus is an instance of the `Promise` object type. — Mike 'Pomax' Kamermans, Oct 16 '21 at 17:34
Instead of doing the thenable check yourself, just call `Promise.resolve(p)` — Bergi, Oct 16 '21 at 18:23
@Mike'Pomax'Kamermans "*anything that's a promise, is a promise, and thus is an instance of the `Promise` object type.*" - [not quite](https://stackoverflow.com/q/29435262/1048572), no. — Bergi, Oct 16 '21 at 18:26
@Bergi fair enough. Is there a prototype symbol that's easy to check at least? (like for Iterables)? Because this shouldn't require duck-typing. — Mike 'Pomax' Kamermans, Oct 16 '21 at 23:07
@Mike'Pomax'Kamermans There would be - if promises weren't predating symbols :-) — Bergi, Oct 17 '21 at 00:49

score 2 · Accepted Answer · answered Oct 16 '21 at 16:37

This happens because in the following line, you call .catch on another (chained) promise, not on the original promise:

  p.then(resolve).catch(reject);

Note how the then call does not get a reject callback argument, and so there is an extra -- intermediate asynchronous promise cycle. By the time the .catch callback is called (reject), the other case has already been dealt with (the fulfulled promise).

So change to:

  p.then(resolve, reject);

Why am I getting an unexpected behavior in my implementation of Promise.race()?

1 Answers1