Add networkTimeoutSeconds to NetworkOnly strategy

[PetrToman]

At present, it is possible to set timeout only for NetworkFirst strategy:

workbox.strategies.networkFirst({networkTimeoutSeconds: 10});

When NetworkOnly strategy is used to retrieve non-cacheable data, it keeps waiting indefinitely in case of a network issue.

Suggested implementation: JakeChampion/fetch#175 (comment)

[jeffposnick]

Fair enough—and in that scenario, you'd be fine with a NetworkError just being raised if the timeout occurs, since there's nothing to fallback on?

[PetrToman]

@jeffposnick Yes. Imho it's better to let the user know something went wrong in a reasonable time than just keep some progress indicator spinning forever :-) Example:

function timeoutPromise(seconds, promise) {
    return new Promise(function (resolve, reject) {
        var timeoutId = setTimeout(function() {
            reject(new Error('timeout'))
        }, seconds * 1000);

        promise.then(
            function(res) {
                clearTimeout(timeoutId);
                resolve(res);
            },
            function(err) {
                clearTimeout(timeoutId);
                reject(err);
            }
        );
    })
}

// Service worker setup
self.addEventListener('fetch', function(e) {
    // don't use Workbox for POST/AJAX requests
    if (e.request.method === 'POST' || /\/someAjaxPath\//.test(e.request.url)) {
        e.respondWith(timeoutPromise(30, fetch(e.request)));
    }
});

// in Ajax handler: a common bit of guesswork to identify timeout
// (I haven't found a way to pass timeout info to the error handler)
error: function(xhr, status) {
    if (xhr.readyState === 4 && xhr.status < 200) alert('Network timeout');
}

[jeffposnick]

keep some progress indicator spinning forever :-)

Just to be clear, though, there really should be a reasonable timeout imposed by the user agent by default. If the current per-browser defaults aren't reasonable, that's good feedback to pass along to browsers that wait too long.

I'm happy to have folks turn to Workbox to give them more control when they want to override those defaults for specific types of requests, but obviously not everyone is going to use Workbox, and a service worker will likely not be in control during the user's first visit to a site.

[PetrToman]

@jeffposnick In case of PWA, a service worker intercepts all network calls (including old fashioned XHRs), which means Fetch API (that has no timeout support - unlike XHR) cannot be easily avoided. Therefore networkTimeoutSeconds option would be useful, I believe.

[PetrToman]

Here's an example of more proper implementation - which aborts timeouted requests, if possible:

// see https://github.com/mo/abortcontroller-polyfill/blob/master/src/utils.js
var isAbortControllerAvailable = 
    !!(typeof(Request) === 'function' && Request.prototype.hasOwnProperty('signal') && AbortController);

function getTimeoutingFetch(seconds, request) {
    return new Promise(function (resolve, reject) {
        var timeoutId = setTimeout(function() {
            reject(new Error('timeout'))
        }, seconds * 1000);

        fetch(request).then(
            function(res) {
                clearTimeout(timeoutId);
                resolve(res);
            },
            function(err) {
                clearTimeout(timeoutId);
                reject(err);
            }
        );
    })
}

function getAbortingFetch(seconds, request) {
    var controller = new AbortController();
    var signal = controller.signal;

    var timeoutId = setTimeout(function() {
        controller.abort();
    }, seconds * 1000);

    var promise = fetch(request, {signal});

    promise.then(
        function(res) {
            clearTimeout(timeoutId);
        },
        function(err) {
            clearTimeout(timeoutId);
        }
    );

    return promise;
}

function getFetch(seconds, request) {
    return isAbortControllerAvailable? getAbortingFetch(seconds, request): getTimeoutingFetch(seconds, request);
}

// Service worker setup
self.addEventListener('fetch', function(e) {
    // don't use Workbox for POST/AJAX requests
    if (e.request.method === 'POST' || /\/someAjaxPath\//.test(e.request.url)) {
        e.respondWith(getFetch(30, e.request));
    }
});

[Rainrider]

Just to be clear, though, there really should be a reasonable timeout imposed by the user agent by default. If the current per-browser defaults aren't reasonable, that's good feedback to pass along to browsers that wait too long.

Chrome on mobile takes 5 minutes(!) to display the offline dino when mobile data is off or flight mode is on. This is probably good as it allows to test for poor connection. The fetch call does not throw in this case though. This makes NetworkFirstOnly strategies unable to utilize the background sync plugin.

My specific use case is about POST requests. Being able to cancel the request if the timeout occurred is crucial. According to caniuse.com Safari 11-12 is the only browser supporting ServieWorkers that has issues with AbortController. Not sure what a solution would be in this case.

Also navigation requests have special handling in workbox currently (see #1796 and #1862) which would make the use of the AbortController fail silently in this case.