tc39 / proposal-await-dictionary

A proposal to add Promise.ownProperties(), Promise.fromEntries() to ECMAScript
MIT License
86 stars 4 forks source link

What should we do if a key or value promise rejects? (Error messaging) #5

Open ajvincent opened 3 years ago

ajvincent commented 3 years ago

Spinoff of #4, and something I definitely haven't thought about. For Promise.fromEntries(), we can at least identify through a .catch() transformation which property is broken. For Promise.ownProperties(), I really don't know, especially if we allow symbols as keys.

ajvincent commented 3 years ago

AggregateError may come into play here.

ljharb commented 3 years ago

I'm not sure why Symbols would affect it one way or the other.

Essentially, I'm envisioning something like:

async function PromiseObject(obj) {
  const entries = Object.entries(obj); // or `Reflect.ownKeys(obj).map(x => [x, obj[x]])`
  const results = await Promise.all(entries.map(([, v]) => v));
  return Object.fromEntries(results.map((x, i) => [entries[i][0], x]));
}
ajvincent commented 3 years ago

Let me suggest code examples of inputs, and you tell me what the outputs should be. That way we have fixtures we can reference.

Figure 1 input

const objectFixture = {
  one: Promise.resolve(1),
  two: Promise.reject(2), // fails on a value
  three: Promise.resolve(3)
};

const result = await Promise.ownProperties(objectFixture);
console.log(JSON.stringify(result));

Figure 2 input

const entriesFixture = [
  [Promise.resolve("four"), Promise.resolve(4)],
  [Promise.resolve("five"), Promise.reject(5)], // fails on a value
  [Promise.resolve("six"), Promise.resolve(6)],
];

const result2 = await Promise.fromEntries(entriesFixture);
console.log(JSON.stringify(result2));

Figure 3 input

const entriesFixture = [
  [Promise.resolve("four"), Promise.resolve(4)],
  [Promise.reject("five"), Promise.resolve(5)], // fails on a key
  [Promise.resolve("six"), Promise.resolve(6)],
];

const result2 = await Promise.fromEntries(entriesFixture);
console.log(JSON.stringify(result2));
ljharb commented 3 years ago

Figure 1 output (all semantics)

Promise.reject(2)

Figure 1 output (allSettled semantics)

({
  one: { status: 'fulfilled', value: 1 },
  two: { status: 'rejected', reason: 2 },
  three: { status: 'fulfilled', value: 3 },
})

For figure 2 and 3, I don't think that we should be expecting keys to be anything but a PropertyKey - iow, a Promise for a key would reject the entire construct. My understanding is that this proposal is only addressing promise values.

ajvincent commented 3 years ago

Okay, I think I'm beginning to understand. You're writing about the output pre-await. I strongly prefer the allSettled semantics here. This allows for more than one promise to reject. (Update: Though when we await it, I think we should be throwing.)

I keep thinking about this post-await, not the components of the promise itself. That's what has been confusing me here about your comments, and that's the disconnect we're having.

Perhaps it's best if we split this issue further into "structure of the returned promise" and "what happens when we await the promise". You're looking at this from the first perspective, and I'm looking at this from the second.

ajvincent commented 3 years ago

As for figure 2, asynchronous map keys & values are the use case. Figure 2 is a value promise (not a promise value, which could be another point of confusion between us if we're not careful).

ljharb commented 3 years ago

@ajvincent no, this is all post-await. It always returns a Promise; the question is "what is it a promise for".

ljharb commented 3 years ago

Objects can't have object keys though, and a Promise is an object - I really don't understand when you'd have asynchronous object keys. The main use case I see is a Promise-valued dictionary, and wanting to await and receive a non-Promise-valued dictionary.

ajvincent commented 3 years ago

Objects can't have object keys though, and a Promise is an object - I really don't understand when you'd have asynchronous object keys. The main use case I see is a Promise-valued dictionary, and wanting to await and receive a non-Promise-valued dictionary.

You are right about objects, and maybe this is a reason to rename .fromEntries() to .mapFromEntries(), so it can return a Map object. It's a special case situation.

ljharb commented 3 years ago

hm, i missed that in the readme. I don't think it makes sense at all to have something that produces a Map object, only a normal object.

If you have an entries iterable, where both the key and value might be promises, then you can use the existing promise combinators, await the result, and pass it into new Map - i'm not convinced that's harder enough than with the proposal to warrant a language addition.

ajvincent commented 3 years ago

hm, i missed that in the readme. I don't think it makes sense at all to have something that produces a Map object, only a normal object.

If you have an entries iterable, where both the key and value might be promises, then you can use the existing promise combinators, await the result, and pass it into new Map - i'm not convinced that's harder enough than with the proposal to warrant a language addition.

Yeah, you're right - I misspoke in the moment. This requires careful thought.

machineghost commented 1 year ago

I don't understand the "keys" part of the question (promises can't be keys), but for the values, this seems like it should be identical to Promise.all. When I do:

await Promise.all([Promise.resolve(5), Promise.reject(7)])

The error message doesn't tell me "you got an error in the promise with an index of 1" ... it just gives me the error (of 7, in my dummy example). It's my responsibility to debug the code (eg. add .catch to the individual promises) if I want to figure out which promise failed.

Similarly here, while we have a key instead of an index, otherwise the behavior seems like it should be identical.

ajvincent commented 1 year ago

I don't understand the "keys" part of the question (promises can't be keys)

That came from a conversation about Promise.fromEntries(), where we thought we might allow the key of a key-value tuple to be a Promise. We've since veered away from that.