How do we allow users to opt into 'safe' resolution?

[mgaudet]

Just looking through the Firefox codebase by asserting when we get more than 1 argument to resolve, there's an example pattern which can bite us:

 // Return a Promise that resolves when the mutex is free.
  return new Promise(unlock => {
    // Overwrite the mutex variable with a chained-on, new Promise. The Promise
    // we returned to the caller can be called to resolve that new Promise
    // and unlock the mutex.
    sessionFileIOMutex = sessionFileIOMutex.then(() => {
      return new Promise(unlock);
    });
  });

The issue here being that unlock is actually being called as an executor, and is called with two arguments (resolve and reject)

A similar pattern you could imagine seeing would be

return new Promise(resolve => {
  setTimeout(() => someOtherGlobal = new Promise(resolve), 0);
});

Which means mostly that I think we'd want to be cautious about how we decide to opt in; either only take explicit booleans, or only take objects of a particular shape resolve(value, { safeResolveMode: true}))

[bakkot]

Does accidentally getting the safeResolve behavior actually break anything? For this to be a problem you have to also be depending on synchronously triggering side-effects, right?

(and in particular using it as an executor wouldn't do anything different at all unless someone had stuck a .then on Function.prototype or Object.prototype, because the executors are not proxies and do not have getters for .then)

[mhofman]

It's only a problem if you trigger the safe resolve with a different value than you would the regular resolve.

But for opt-in, why are we looking at changing the resolve function itself vs a different path to get the safe resolve.

E.g. new Promise.Safe((safeResolve, reject) => {} ?

[mgaudet]

Does accidentally getting the safeResolve behavior actually break anything? For this to be a problem you have to also be depending on synchronously triggering side-effects, right?

Yeah, it's true. I discovered this while prototyping telemetry and then panicked a bit -- I was definitely tired and a bit over-wired when I filed this.

It certainly could cause a behaviour change, but the general thesis is that the behaviour change is 1) rare, 2) not particularly impactful in the majority of cases.

But for opt-in, why are we looking at changing the resolve function itself vs a different path to get the safe resolve.

E.g. new Promise.Safe((safeResolve, reject) => {} ?

Well, I think the question is "do you want the creator of the promise or the resolver of the promise to choose the resolution behaviour"; I think to provide the safety property we want in general we would like the place where a promise is resolved to be able to choose the safe behaviour; but if you're just handed a promise from user code and you are a library that wants to do safe resolve... you're hooped unless you can make the resolver mode switch, which is why we're talking the second argument.

[bergus]

Forgive me if I missed some discussion, but I have to ask: why do you want to allow user code to opt into this behaviour at all? From how I understood the repo readme, the issue is mainly with native code, so this only needs to be exposed as a new abstract operation for layering with other specs.

Adding a second parameter to resolve would make the Promise API unnecessarily complicated imo. What kind of code would you expect to use this parameter? If this is a safety/security issue, having "unsafe" mode as the default and having to opt into "safe" mode seems like a bad idea; it will often be overlooked or forgotten. Authors who know and care about the issue can already write Promise.resolve().then(() => x) instead of Promise.resolve(x), so nothing changes for them.

[bakkot]

User code also likes to be able to reason about side effects. I expect you'd want to use this pretty much any time you were resolving a Promise with a value not expected to be thenable.

I agree it would be better to change the default behavior but that would not be web compatible.

[bergus]

any time you were resolving a Promise with a value not expected to be thenable

Yes, but in those cases what you really want is the capability to fulfill the promise. Not resolveButOnlyCheckThenabilityInALaterTick ("safe resolve").
The API I'd envision for that would be .fulfill and .reject methods on the resolve function:

new Promise(resolve => { resolve.fulfill(anyValue); /* or */ resolve.reject(error); /* or */ resolve(thenable); })
new Promise(({ fulfill, reject }) => { fulfill(anyValue); /* or */ reject(error); })

That's much more straighforward and - still being opt-in - I believe also web compatible. By no longer checking thenability at every resolve point it should also allow better engine optimisations.

[bakkot]

I was in favor but some people were opposed to that; see e.g. #5.