Search code examples
javascriptpromisees6-promise

Am I chaining Promises correctly or committing a sin?


I have not worked with Javascript in a long time, so now promises are a new concept to me. I have some operations requiring more than one asynchronous call but which I want to treat as a transaction where steps do not execute if the step before failed. Currently I chain promises by nesting and I want to return a promise to the caller.

After reading the chaining section of Mozilla's Using Promises guide, I'm not sure if what I'm doing is correct or equivalent to the "callback pyramid of doom".

Is there a cleaner way to do this (besides chaining with a guard check in each then)? Am I right in my belief that in Mozilla's example it will execute each chained then even when there is an error?

myfunction(key) => {
    return new Promise((outerResolve, outerReject) => {
        return new Promise((resolve, reject) => {
            let item = cache.get(key);
            if (item) {
                resolve(item);
            } else {
                //we didnt have the row cached, load it from store   
                chrome.storage.sync.get(key, function (result) {
                    chrome.runtime.lastError
                        ? reject({ error: chrome.runtime.lastError.message })
                        : resolve(result);
                });
            }
        }).then((resolve) => {
            //Now the inner most item is resolved, we are working in the 'outer' shell
            if (resolve.error) {
                outerReject(resolve);
            } else {
                //No error, continue
                new Promise((resolve, reject) => {
                    chrome.storage.sync.get(keyBasedOnPreviousData, function (result) {
                        chrome.runtime.lastError
                            ? reject({ error: chrome.runtime.lastError.message })
                            : resolve(result);
                    });
                }).then((resolve) => {
                    //finally return the result to the caller
                    if (resolve.error) {
                        outerReject(resolve);
                    } else {
                        outerResolve(resolve);
                    }
                });
            }
        });
    });
}

Solution

  • Subsequent then statements are not executed (until a catch) when an exception is thrown. Also, .then returns a Promise, so you don't need to create an additional, outer Promise.

    Try this example:

    var p = new Promise((resolve, reject) => {
        console.log('first promise, resolves');
        resolve();
    })
    .then(() => {
        throw new Error('Something failed');
    })
    .then(() => {
        console.log('then after the error');
        return('result');
    });
    
    p.then(res => console.log('success: ' + res), err => console.log('error: ' + err));
    

    You will not see "then after the error" in the console, because that happens after an exception is thrown. But if you comment the throw statement, you will get the result you expect in the Promise.

    I am not sure I understand your example entirely, but I think it could be simplified like this:

    myfunction(key) => {
        return new Promise((resolve, reject) => {
            let item = cache.get(key);
            if (item) {
                resolve(item);
            } else {
                //we didnt have the row cached, load it from store   
                chrome.storage.sync.get(key, function (result) {
                    chrome.runtime.lastError
                        ? throw new Error(chrome.runtime.lastError.message)
                        : resolve(result);
                });
            }
        }).then((previousData) => {
            // keyBasedOnPreviousData is calculated based on previousData
            chrome.storage.sync.get(keyBasedOnPreviousData, function (result) {
                chrome.runtime.lastError
                    ? throw new Error(chrome.runtime.lastError.message)
                    : return result;
            });
        });
    }