Reject networkFirst when there's a network failure + cache miss

[jeffposnick]

R: @gauntface @wibblymat @addyosmani

This closes #80 by ensuring that networkFirst rejects when the following conditions are met:

• There's no previously cached entry to return.
• The initial network request rejected (as opposed to resolved with an error Response).

Prior to this PR, networkFirst would resolve with undefined when those conditions were met. This PR bring the networkFirst behavior in line with the default fetch() behavior.

I'm very much tempted to add a test case for this scenario, but I don't know how to properly force a NetworkError in response to a fetch(). Maybe once there's some precedent for mocking in our tests that could be revisited.

[gauntface]

There are tests along those lines in the branch the original tests were
written to, they just haven't made it to master yet. Happy to add a test
case separately from this PR.

On Wed, 20 Jan 2016, 18:59 Jeffrey Posnick notifications@github.com wrote:

R: @gauntface https://github.com/gauntface @wibblymat
https://github.com/wibblymat @addyosmani https://github.com/addyosmani

This closes #80 #80 by
ensuring that networkFirst rejects when the following conditions are met:

• There's no previously cached entry to return.
• The initial network request rejected (as opposed to resolved with an
  error Response).

Prior to this PR, networkFirst would resolve with undefined when those
conditions were met. This PR bring the networkFirst behavior in line with
the default fetch() behavior.

I'm very much tempted to add a test case for this scenario, but I don't
know how to properly force a NetworkError in response to a fetch(). Maybe
once there's some precedent for mocking in our tests that could be

revisited.

You can view, comment on, or merge this pull request online at:

#84
Commit Summary

• networkFirst should reject when there is no response
• Build artifacts

File Changes

• M lib/strategies/networkFirst.js
  https://github.com/GoogleChrome/sw-toolbox/pull/84/files#diff-0 (17)
• M sw-toolbox.js
  https://github.com/GoogleChrome/sw-toolbox/pull/84/files#diff-1 (10)
• M sw-toolbox.map.json
  https://github.com/GoogleChrome/sw-toolbox/pull/84/files#diff-2 (2)

Patch Links:

• https://github.com/GoogleChrome/sw-toolbox/pull/84.patch
• https://github.com/GoogleChrome/sw-toolbox/pull/84.diff

—
Reply to this email directly or view it on GitHub
#84.

[addyosmani]

This closes #80 by ensuring that networkFirst rejects when the following conditions are met: There's no previously cached entry to return. The initial network request rejected (as opposed to resolved with an error Response).

The updated behaviour LGTM. Requires rebase.

[wibblymat]

I held off LGTMing this for ages because of the old "is it really an exception case?" dilemma. But 5 days is long enough to convince me that I don't have a strong enough opinion to challenge it.

LGTM!

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

[googlebot]

CLAs look good, thanks!

[AaronLayton]

If I update in Bower will I get this update now?

[jeffposnick]

@AaronLayton, feel free to pull in GoogleChrome/sw-toolbox#master in your bower.json to test out the updated behavior. We haven't tagged a formal release for this yet.

@wibblymat, I think the fact that the Fetch API treats this type of failure as an exception means that it's a little more clear-cut as to what sw-toolbox should do. We're not translating an error Response to an exception here; we're just passing along an exception that was triggered by the underlying fetch().