0

I am wondering how can I properly break promise chain in JS.

In this code, I am connecting to database at first, then I am checking if collection already has some data, if not add them. Don't pay attention to some actionhero.js code..it does not matter here.

The main question is: Is it okay to break chain using throw null?

mongoose.connect(api.config.mongo.connectionURL, {})
        .then(() => {
            return api.mongo.City.count();
        })
        .then(count => {
            if (count !== 0) {
                console.log(`Amout of cities is ${count}`);
                throw null; // break from chain method. Is it okay ?
            }
            return api.mongo.City.addCities(api.config.mongo.dataPath + '/cities.json');
        })
        .then(cities => {
            console.log("Cities has been added");
            console.log(cities);
            next();
        })
        .catch(err => {
            next(err);
        })

Many thanks!

Eduard Bondarenko
  • 363
  • 2
  • 14
  • 1
    No, you should always throw an `Error` instance. In particular, never do `thow null`, as that would call `next` without an `err` – Bergi May 18 '16 at 10:35
  • You also call next() without an err in then callback. So what the problem ? – Eduard Bondarenko May 18 '16 at 14:36
  • But it does suggest a success result, which doesn't seem to be the case? Even if that is the expected behaviour, it's totally misleading. – Bergi May 18 '16 at 17:09
  • 1
    Why not `return [];` instead? – loganfsmyth May 18 '16 at 18:24
  • Because chain may contain more than one or two cases..if you return []; you should check that result is empty in every then case. – Eduard Bondarenko May 22 '16 at 04:48
  • For your title question, see [How to properly break out of a promise chain?](http://stackoverflow.com/q/29499582/1048572) – Bergi May 29 '16 at 22:19
  • See https://stackoverflow.com/questions/28803287/how-to-break-promise-chain/45339587#45339587 Use `return { then: function() {} };` – Martin Jul 27 '17 at 01:03

1 Answers1

2

Despite it may seem like a clever trick and will work as You expect, I would advise against throwing non-error objects.

It would be much more predictable for other developers that will maintain this code if You throw an actual Error and handle it explicitly.

Promise
  .resolve()
  .then(() => {
    throw new Error('Already in database');
  })
  .catch(err => {
    if (err.message === 'Already in database') {
      // do nothing
    } else {
      next(err);
    }
  });
Роман Парадеев
  • 4,186
  • 19
  • 35