AngularJS - mixing HTTP and custom promises with recursion

Question

I have written a function wrapper that returns cached values for HTTP responses. In a specific situation (marked by comment // <--HERE) I see inconsistent behavior. I'm frankly not sure what exactly the inconsistency is, but bottom line, when the cache expires (has_expired), it does not wait for the http get to return in the recursive call.

My guess is I haven't put a "return" somewhere on a promise but I can't find out where (and why). Do I need to put a return in front of localForage.removeItem (and if so why?)

function cache_or_http(url,key) {

    if (dont_use_cache()) {
      return $http.get(url);
    }
    var d = $q.defer();
    localforage.getItem(key)
    .then (function(data) {
         if (data) { // exists
            if (has_expired(data.created_at)) {
                localforage.removeItem(key)
                .then (function() {return cache_or_http(url,key);}) // <--HERE
                .catch(function() {return do_error_handling();})
            } else { // not expired
                 d.resolve(JSON.parse(data.value));
                 return d.promise;
             }
         } else {
            // doesn't exist
            return $http.get(url)
            .then (function(data) {
                cache_entry = {
                    'value': JSON.stringify(data),
                    'created_at': moment().toString()
                };
                localforage.setItem(key, cache_entry);
                d.resolve(data);
                return (d.promise);
            });
         } // doesn't exist
    }); // getItem .then
    return (d.promise);
 }

georgeawg · Accepted Answer · 2020-10-23 01:38:01Z

1

There is no need to manufacture a new promise with $q.defer. The .then method of a promise already returns a promise.

function cache_or_http(url,key) {
    ̶v̶a̶r̶ ̶d̶ ̶=̶ ̶$̶q̶.̶d̶e̶f̶e̶r̶(̶)̶;̶
    ̲r̲e̲t̲u̲r̲n̲ localforage.getItem(key)
    .then (function(data) {
         if (data) { // exists
            if (has_expired(data.created_at)) {
                ̲r̲e̲t̲u̲r̲n̲ localforage.removeItem(key)
                .then (function() {return cache_or_http(url,key);}) // <--HERE
                .catch(function() {return do_error_handling();})
            } else { // not expired
                 ̶d̶.̶r̶e̶s̶o̶l̶v̶e̶(̶J̶S̶O̶N̶.̶p̶a̶r̶s̶e̶(̶d̶a̶t̶a̶.̶v̶a̶l̶u̶e̶)̶)̶;̶ 
                 return JSON.parse(data.value);
            }
         } else {
            // doesn't exist
            return $http.get(url)
            .then (function(data) {
                cache_entry = {
                    'value': JSON.stringify(data),
                    'created_at': moment().toString()
                };
                ̲r̲e̲t̲u̲r̲n̲ localforage.setItem(key, cache_entry);
                ̶d̶.̶r̶e̶s̶o̶l̶v̶e̶(̶d̶a̶t̶a̶)̶;̶
                ̶r̶e̶t̶u̶r̶n̶ ̶(̶d̶.̶p̶r̶o̶m̶i̶s̶e̶)̶;̶
            });
         } // doesn't exist
    }); // getItem .then
    ̶r̶e̶t̶u̶r̶n̶ ̶(̶d̶.̶p̶r̶o̶m̶i̶s̶e̶)̶;̶
 }

For more information, see

Is this a "Deferred Antipattern"?

answered Oct 23, 2020 at 1:38

georgeawg

49k13 gold badges78 silver badges98 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

user1361529 Over a year ago

Thank you. I understand I was needlessly wrapping things around additional promises. For my understanding, putting aside the part about additional wraps, is there an error with my code?

user1361529 Over a year ago

I've also added a small if situation in my original code (no cache setting) that further confuses me - I think in this case, I do need my own promise wrapper?

user1361529 Over a year ago

Okay, I wrote up a fiddle using your suggestion jsfiddle.net/mj9vchnr/11 - seems to work fine, but in my own code, I'm facing issues. I'll need to dive in deeper into what is going on. Thanks.

user1361529 Over a year ago

found the issue - was in my code, unrelated to your suggestion

user1361529 Over a year ago

Do I need to make sure the first line in the function is a return? Please see my modified code. It works but I’ve read the entire code should be wrapped in return.

Collectives™ on Stack Overflow

AngularJS - mixing HTTP and custom promises with recursion

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related