Difference between return Error and throw Error

Question

I found the following code in a project, that I do not understand.:

get(key, store = null) {
    if (!key) {
      return new Error('There is no key to get!');
    }

    let dbstore = this.localforage;

    if (store !== null) {
      dbstore = store;
    }

    return dbstore
      .getItem(key)
      .then(function(value) {
        return value;
      })
      .catch(function(err) {
        return new Error('The key (' + key + ") isn't accessible: " + err);
      });
  }

Why return new Error('There is no key to get!'); instead of throw new Error('There is no key to get!');?

Also why not throw an error in the catch block?

Solution

When you design a function interface and there are errors to deal with, you have a design choice for how to return errors. If the function is synchronous, you can either return some sentinel value that indicates an error and is easily distinguished from an actual result (often null in Javascript) or you can throw an exception or you can return an object that has a property that indicates the success or failure of the operation.

When you have an asynchronous operation with a promise interface, one would usually reject the Promise with an Error object as the reject reason to signify an error. That's the core design theory of promises. Success resolves with an optional value, errors reject with a reason.

This block of code:

return dbstore
  .getItem(key)
  .then(function(value) {
    return value;
  })
  .catch(function(err) {
    return new Error('The key (' + key + ") isn't accessible: " + err);
  });

is resolving the returned promise with either a value or an Error object. This is generally not how promise code is written because it will require the caller to test the type of the resolved value to figure out if there's an error or not which is not the simple, straightforward way to use promises. So, to your question, you would usually do this:

return dbstore.getItem(key).catch(function(err) {
    throw new Error('The key (' + key + ") isn't accessible: " + err);
});

There are other signs in this function, that it's just bad code.

1. .then(function(value) {return value;}) is completely superfluous and unnecessary. It adds no value at all. The value is already the resolved value of the promise. No need to declare it again.

2. The function sometimes returns a promise and sometimes throws a synchronous exception.
   This is even a further pain to use. If you look at the first if (!key) { statement, it returns an Error object is the key argument isn't supplied. That means that to use this function you have to catch synchronous exceptions, provide .then() and .catch() handlers AND check the type of the resolved promise to see if it happens to be an error object. This function is a nightmare to use. It's bad code.

To use the function as it is, the caller would likely have to do this:

let retVal = someObj.get(aKey);
if (typeof retVal === Error) {
    // got some synchronous error
} else {
    retVal.then(val => {
        if (typeof val === Error) {
            // got some asynchronous error
        } else {
            // got an actual successful value here
        }
    }).catch(err => {
        // got some asynchronous error
    })
}

The function implementation probably should be this:

get(key, store = null) {
    if (!key) {
        return Promise.reject(new Error('There is no key to get!'));
    }

    let dbstore = store || this.localforage;

    return dbstore.getItem(key).catch(function(err) {
        throw new Error('The key (' + key + ") isn't accessible: " + err);
    });
}

This can then be used like this:

someObj.get(aKey).then(val => {
    // got some successful value here
}).catch(err => {
    // got some error here
});

Compare the simplicity for the caller here to the mess above.

This implementation has these consistencies:

1. It always returns a promise. If key isn't supplied, it returns a rejected promise.
2. All errors come via a rejected promise
3. The value the promise resolves with is always an actual successful value
4. There's no .then() handler that does nothing useful.