[Bug 24291] Add a Promise type to WebIDL, and make it not distinguishable from anything

https://www.w3.org/Bugs/Public/show_bug.cgi?id=24291

--- Comment #9 from Tab Atkins Jr. <jackalmage@gmail.com> ---
(In reply to Domenic Denicola from comment #8)
> Tab, would you mind sharing the specific example where you think unioning is
> useful?

Sure!  I'll do so below.

> To be clear, the plan is that an API accepting a promise will take anything
> you give it and run it through `Promise.cast` before processing it. So you
> kind of automatically get "unioning" of a sort: you can pass in any
> non-promise value, and it becomes (via WebIDL overload resolution) a promise
> for that value.
> 
> Is that what you had in mind, or is it something different?

That's acceptable, but not ideal.  It implies consing up a fresh Promise that
is then immediately and silently consumed by the API.  On the other hand, it
keeps you honest and prevents you from designing different behavior in the
sync/async argument codepaths.

So, the example.

When a page with an installed Service Worker makes a request, we shoot a
"fetch" event at the SW to allow it to intercept the request.  To do so, the SW
has to at some point call "event.respondWith(...)", with ... being either a
Response object or a Promise<Response>.  The latter is what's returned by
closely-related APIs like the Cache objects, so it's rather convenient.  This
means you can just write code like:

```
e.respondWith(cache.get(e.requestURL))
```

You might think that you can just invert that and do:

```
cache.get(e.requestURL).then(e.respondWith.bind(e))
```

But you can't!  (Also, note that this line is more complex by a bit anyway, due
to the necessary binding.)  Or at least, it's not identical behavior.

You need to signal *within the "fetch" event* that you're going to handle the
response.  If you don't, the UA will let the next "fetch" listener handle it,
or just send it to the network if there's no one else. So you have to augment
the code to:

```
e.preventDefault();
e.stopImmediatePropagation();
cache.get(e.requestURL).then(e.respondWith.bind(e))
```

But this still isn't right!  In the original form, if the cache promise
*rejects* (because there's nothing in the cache for that URL), the fetch will
immediately fail with a network error.  In our latest code, a rejection will
just do nothing, and so the fetch will spin until timeout.  (This doesn't have
severely negative effects, but it does mean that you'll see a loading spinner
in your tab for much longer than you should.)

To fix it, you need to signal that you're done dealing with the response in the
error case too:

```
e.preventDefault();
e.stopImmediatePropagation();
cache.get(e.requestURL).then(e.respondWith.bind(e), e.respondWith.bind(e))
```

(This'll send whatever the failure value is to respondWith(). Since it's not a
Response, the SW will treat this as a network error.)

Our code is now equivalent in functionality.  Let's compare it again to the
original code:

```
e.respondWith(cache.get(e.requestURL));
```

Considering this is pretty much the simplest-possible async-response case, and
this or something very close to it will be very common, the increase in
complexity is rather large and frankly untenable.  Failing to do everything
right will result in bad behavior *sometimes* (the promise resolution will race
with other fetch handlers (microtasks preempt queued events!); cache failures
will create long-running spinners; etc).

(Note that some smallish changes can make it less horrible. We can add a
shorthand to the event to claim handling, and assume that .finally() is on
promises, so we'd write it as:

```
e.stop();
cache.get(e.responseURL).finally(e.respondWith.bind(e));
```

But this is still a significant increase in complexity, and not the most
obvious way to write it.)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Received on Thursday, 23 January 2014 02:54:46 UTC