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

Promise handling algorithms are not very clear about what happens if type conversion fails #782

Open
domenic opened this issue Aug 29, 2019 · 6 comments · May be fixed by #791
Open

Promise handling algorithms are not very clear about what happens if type conversion fails #782

domenic opened this issue Aug 29, 2019 · 6 comments · May be fixed by #791

Comments

@domenic
Copy link
Member

domenic commented Aug 29, 2019

https://heycam.github.io/webidl/#dfn-perform-steps-once-promise-is-settled

What if the type is Promise<Node> and someone passes Promise.resolve(3)?

I think the best we can do is go down the rejection path? Although that makes using "upon fulfillment" in isolation pretty dangerous for non-any _T_s.

@domenic
Copy link
Member Author

domenic commented Aug 29, 2019

I am curious if implementations actually pay attention to the T at all... Do they have counterpart algorithms?

@bzbarsky
Copy link
Collaborator

bzbarsky commented Aug 29, 2019

Gecko pays attention to the T in the following ways:

  1. IDL parse fails if Promise<T> is the return type of an operation or attribute but T is not exposed in all the globals where the operation or attribute is exposed. There is no corresponding requirement in the spec, as far as I can tell (also for the situation where some type is returned directly, without Promise being involved), but fundamentally it doesn't make sense to have this setup.
  2. Something involving which C++ headers to include in generated binding files. I'm not even convinced that code needs to be there.
  3. The equality operator on types in the IDL parser treats Promise<T> as equal to Promise<U> if and only if T == U. This equality operator is used when coalescing identical unions across IDL files. It could probably ignore the parameter type and just treat app Promise as equal in practice...

There are no other places that are aware of the T in Gecko as far as I can tell.

@annevk
Copy link
Member

annevk commented Aug 30, 2019

How does respondWith() work in Gecko? Is there a manual type check?

@Ms2ger
Copy link
Member

Ms2ger commented Aug 30, 2019

In the spec as written, it'll return a rejected promise and the success steps won't be run.

Let's take a look at what APIs take a Promise argument anyway:

This suggests to me that we should run the rejection steps on conversion failure.

@bzbarsky
Copy link
Collaborator

How does respondWith() work in Gecko? Is there a manual type check?

Yes. See https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/serviceworkers/ServiceWorkerEvents.cpp#576,588-591,595-597,605-608 (which checks that the resolution value is an object, then checks that it's a Response.

@domenic
Copy link
Member Author

domenic commented Aug 30, 2019

In the spec as written, it'll return a rejected promise and the success steps won't be run.

Ah, I guess because "converting" will throw, and PerformPromiseThen handles it. That makes sense.

This suggests to me that we should run the rejection steps on conversion failure.

Sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants