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

Use of Upon fulfillment/rejection in CacheStorage.match is incorrect #627

Closed
jyasskin opened this issue Feb 17, 2015 · 4 comments
Closed

Comments

@jyasskin
Copy link
Member

https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#cache-storage-match-method has:

  1. Return the result of transforming ... with a fulfillment handler that ...:
    1. For each key in keys:
      1. Let q be [a promise returned by ...]
      2. Upon fulfillment of q with value matchedResponse:
        1. If matchedResponse is not undefined, return matchedResponse.
      3. Upon rejection of q with value err:
        1. Throw err.
    2. Return undefined.

Upon Fulfillment/Rejection handlers call q.then, but they don't do anything with the returned promise, so the values returned and thrown in those handlers don't wind up getting used. This Else branch is always going to fulfill with undefined, which probably wasn't intended.

@jungkees
Copy link
Collaborator

Corrected the steps: 7751742. Now it intends to resolve with the first matching response (from the Cache object iterated in the key insertion order) from the promise chain. It resolves with undefined when there's no match.

@jyasskin
Copy link
Member Author

50b805f changes the steps, but I think they leave p referring to the last matching response rather than the first. I don't see anything breaking out of the 'for each'.

You also don't need a rejection handler that throws its argument. That's the default.

@jungkees
Copy link
Collaborator

'For each' just builds the promise chain. And when one of the fulfillment handlers in the chain returns a promise which eventually resolves with a non-undefined value (i.e. a matched Response object), it makes all the following fulfillment handlers return the same value till the last one. So I think the first matched response will be the final value for the returned promise. Am I missing something?

You also don't need a rejection handler that throws its argument. That's the default.

Yes. It's removed: 37064b6.

@jyasskin
Copy link
Member Author

Aha! Yes, "If v is not undefined, return v." covers this. Thanks!

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

No branches or pull requests

2 participants