How to avoid manually caching a request that returned an error?

Question

I've created a module (following Javascript's Module Pattern) that makes an http request, caches and returns the result:

var requestService = (function($) {

    var cache = {};

    var get = function(date) {
        var params = date ? {'date': date} : {};
        return $.getJSON('http://my/url', params, function( result ){});
    };

    var fetchData = function(date) {
        if (!cache[date]) {
            cache[date] = get(date);
        }

        return cache[date].done(function(myData) {
            return new Promise(function(resolve,reject) {
                resolve(myData);
            });
        });
    };

    return {
        fetchData: fetchData
    };
})(jQuery);

My problem is that the results are cached even if there's an error (ex: the host is temporary unreachable). I don't want that to happen.

I thought that the done() function would only be called if the request succeeded, but it's not the case. Should I add a cache[date].fail(), setting itself to null? What I mean:

return cache[date].done(function(myData) {
    return new Promise(function(resolve,reject) {
        resolve(myData);
    });
}).fail(function(myData) {
    cache[date] = null;
    return new Promise(function(resolve,reject) {
        reject(myData);
    });
});

Here's how my requestService is called in another module:

requestService.fetchData(myDate).done(function(myData) {
    // code when successful
}).fail(function(d, textStats, error) {
    // error
});

Landfall

Answer 1

done and fail do not support chaining callback results, your return new Promise is absolutely pointless. Also it looks as if you are trying to use the Promise constructor antipattern. What your code does it to simply return the ajax promise as-is (which works, as you can chain onto it).

the results are cached even if there's an error

Yes - by storing the promise, the whole request result is cached, not only in the success case.

Should I add a rejection handler, setting itself to null?

Yes, that's exactly what you'll need to do:

var requestService = (function($) {
    var cache = {};

    function get(date) {
        var params = date ? {'date': date} : {};
        return $.getJSON('http://my/url', params);
    }

    function fetchData(date) {
        if (!cache[date]) {
            cache[date] = get(date);
            cache[date].fail(function(err) {
                cache[date] = null; // retry next time
            });
        }
        return cache[date];
    }

    return {
        fetchData: fetchData
    };
})(jQuery);

If you want to return a native promise, use

function fetchData(date) {
    if (!cache[date]) {
        cache[date] = Promise.resolve(get(date)).catch(function(err) {
            cache[date] = null; // retry next time
            throw err;
        });
    }
    return cache[date];
}

Uranus

thanks for the heads up! I am indeed new to the promise API, this code is much cleaner. – Landfall

Answer 2

ok nevermind, that code worked with some adjustments. In my fetchData method I'm checking if the object is empty (so undefined or null) and I've added a fail() method that nullifies the cache[date]. I do the following now:

var requestService = (function($) {

    var cache = {};

    var get = function(date) {
        var params = date ? {'date': date} : {};
        return $.getJSON('http://my/url', params, function( result ){});
    };

    var fetchData = function(date) {
        // now also checks for null
        if ($.isEmptyObject(cache[date])) {
            cache[date] = get(date);
        }

        return cache[date].done(function(myData) {
            return new Promise(function(resolve,reject) {
                resolve(myData);
            });
        }).fail(function(myData) {
        return new Promise(function(resolve,reject) {
            // nullifies cache[date]
            cache[date] = null;
            reject(myData);
        });
    };

    return {
        fetchData: fetchData
    };
})(jQuery);

Landfall