How to avoid spaghetti code when using nested promises?

Question

Say you have 6 methods that return promises, A(), B(), C(), D(), E(), F() and G().

Several of them cannot execute properly until others have. For instance, C() requires A(). F() & E() require C(). B() requires A() and F(). G() requires all others to be complete.

A().then(() => {
  C().then(() => {
    F().then(() => {
      B();
    });
    E();
  });
}).then(() => {
  G();
});
D();

Now, say that some logic needs to be run between several of these functions that affect their behavior.

A().then(() => {
  let foop = this._service.getFoop();
  C(foop).then((noop) => {
    noop.forEach((n) => { n *= -1; });
    F(noop).then((boop) => {
      boop = boop.flatten();
      B(foop, boop);
    });
    E('MonkeyWrench');
  });
}).then(() => {
  let glipglop = this._service.findGlip(this.LOCAL_GLOP_KEY);
  G(glipglop, 42);
});
D();

And then, on top of that, we need to make sure that each of these promises catches and handles errors.

A().then(() => {
  let foop = this._service.getFoop();
  C(foop).then((noop) => {
    noop.forEach((n) => { n *= -1; });
    F(noop).then((boop) => {
      boop = boop.flatten();
      B(foop, boop).catch((e) => {
        this.handleErrorB(e);
      });;
    }).catch((e) => {
      this.handleErrorF(e);
    });
    E('MonkeyWrench').catch((e) => {
      this.handleErrorE(e);
    });
  }).catch((e) => {
    this.handleErrorC(e);
  });
}).then(() => {
  let glipglop = _service.findGlip(this.LOCAL_GLOP_KEY);
  G(glipglop, 42);
}).catch((e) => {
  this.handleErrorA(e);
});
D().catch((e) => {
  this.handleErrorD(e);
});

So now we have a complete async program made up of promises - and it looks like a complete mess.

What is a way to take a program like this and make it easier to read so that future contributors can avoid diving into spaghetti code? Are nested promise structures inherently messy, or are there accepted protocols to clean this up and make it more readable?

I read http://www.datchley.name/promise-patterns-anti-patterns/ and it had some good ideas such as using promise.all([...]) to run multiple promises at once, but it didn't include any support for having logic between nested promise calls.

Thanks in advance to anyone willing to impart some wisdom for me on this.

Do you really want to run them in parallel? It looks like you have forgotten several `return`s. — Bergi, Apr 23 '17 at 23:02
What do your error handlers actually do? Do you want to continue the flow after them (because the supply default results etc)? And do you want them to handle errors not only in the promised results by the functions with the respective name, but also in the handlers chained to them? — Bergi, Apr 23 '17 at 23:06
Please post your real code. It's very hard to see what could be improved without destroying your logic. — Bergi, Apr 23 '17 at 23:06
The [async/await pattern](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await) can be used to solve this mess... — Jordão, Apr 23 '17 at 23:25
@Jordão Can you try to show that in an answer? I don't think applying it will clean up a lot here. — Bergi, Apr 23 '17 at 23:39

score 2 · Accepted Answer · edited May 23 '17 at 12:09

The Promise.all([…]) pattern works especially fine with logic between the promise calls - just create a new promise for the new value!

var a = A();
var foop = a.then(() => this._service.getFoop());
var c = foop.then(C);
var noop = c.then(x => x.map(n => n * -1));
var f = noop.then(F);
var boop = f.then(x => x.flatten());
var b = Promise.all([boop, foop]).spread(B);
var e = Promise.all([boop, foop]).then(() => E('MonkeyWrench'));
var d = D();
var glipglop = Promise.all([d, e, f]).then(() =>
  this._service.findGlip(this.LOCAL_GLOP_KEY));
return Promise.all([gliplop, 42]).spread(G);

You can add your error handlers to the respective promises.

So now we have a complete async program made up of promises - and it looks like a complete mess.

That's not because of promises, but because your control flow is a mess. Maybe it's inherent complexity, but most likely it's not. Try to factor out parts of the logic into separate functions - a strategy that admittedly works better with the nested closure pattern. Also try to avoid side effects - logic that needs to run at particular times in a concurrent control flow, and affects other functionality, will always be messy. Instead, make it as pure as possible to work on immutable values only that are passed down the flow.

How to avoid spaghetti code when using nested promises?

1 Answers1