How do I conditionally run a .then() function in a .then .catch chain?

Question

My code resembles this:

router.post("/", (req, res, next) => {
foo()
.then((result) => {
    res.send(result)
})
.catch((error) => {
    cosnole.log(error)
})
//I only want the bar() function and everything below it to run if the first promise is rejected and the first .catch function ran
bar()
.then((data) => {
    res.send(data)
})
.catch((error) => {
    console.log(error)
})

})

I'd like to only run the bar() function and the .then .catch functions after it only if the first promise is rejected and the .catch function fires off.

I've tried this:

router.post("/", (req, res, next) => {
rejected = false
foo()
.then((result) => {
    res.send(result)
})
.catch((error) => {
    rejected = true
    console.log(error)
})
if(rejected == true)
    bar()
    .then((data) => {
        res.send(data)
    })
    .catch((error) => {
        console.log(error)
    })

})

but the bar() function never gets executed when the first foo() function's error is caught and the promise is rejected.

your if code has executed before then() gets resolved – guijob Feb 27 '18 at 15:17 — guijob, Feb 27 '18 at 15:17

score 4 · Accepted Answer · edited Feb 27 '18 at 15:22

4

Move the call to inside the catch.

router.post("/", (req, res, next) => {
    foo()
    .then((result) => {
        res.send(result)
    })
    .catch((error) => {
        cosnole.log(error);
        return bar()
        .then((data) => {
            res.send(data)
        })
        .catch((error) => {
            console.log(error)
        });
    });
});

edited Feb 27 '18 at 15:22

Bergi

630,263
148
957
1,375

answered Feb 27 '18 at 15:09

Igor

60,821
10
100
175

Weren't promises supposed to solve the nested callback problem? – Feb 27 '18 at 15:12
1

@doodlemeister Structured programming requires nesting for control flow. And you still can chain onto it by simply returning, [that's the magic of promises](https://stackoverflow.com/a/22562045/1048572). – Bergi Feb 27 '18 at 15:21
@Bergi: Nested functions isn't required, even without promises. The nesting/closures is just a lazy way of managing a collection of data. I don't believe it would take you more than a few seconds to solve this without further nesting. No "magic" needed. – Feb 27 '18 at 15:26
1

@doodlemeister Well for closures we always need at least one level of nesting, but yes we could extract named functions and manage our scope environments explicitly. Writing them inline is just simpler and often more readable. – Bergi Feb 27 '18 at 15:33
@Bergi: Closures aren't really needed, but one level is indeed useful for encapsulating functions that are relevant only to the current operation. If using anonymous callbacks was actually cleaner, we'd be using them for everything. Instead we (at least in most code I see) create named functions to organize our code. I don't see why it should be any different for async code... but that's just me. ;-) – Feb 27 '18 at 15:37
...either way, this falls right back into the original nested callback problem that was the original impetus for creating promises. – Feb 27 '18 at 15:38
@doodlemeister I think you'll have a really hard time doing anything useful in JS without closures. – Bergi Feb 27 '18 at 15:42
@Bergi: I have nothing against closures in general. They, like anything, are useful in moderation. But if one of the main points of promises was to solve the nested callback problem, then we should write code that doesn't fall back into that problem. A named function solves it here, and in general, such techniques can be used to write more natural and idiomatic code than promises allow. – Feb 27 '18 at 15:49
@doodlemeister Promises can only flatten linearly nested callbacks, not conditional ones - and they never were meant to do. The beauty of promises is that all their goodies (error handling, being values) works *even in the presence* of nesting. – Bergi Feb 27 '18 at 15:58
@Bergi: Nesting, being the original problem to be solved by promises, can be avoided here, so why not avoid it? This solution leads us back into that callback pyramid. Anyway, what you see as "goodies", I see as obfuscation. Visible control flow becomes hidden behind abstractions. – Feb 27 '18 at 16:09
@doodlemeister Feel free to post your own answer :-) (And yes, using control flow abstractions from functional programming is one of my passions :P) – Bergi Feb 27 '18 at 16:12
@Bergi: Well, I was hoping Igor would explain why he used a solution that leads back to a problem. Your solution certainly does avoid that, though it may be taking advantage of a simplified example. Hard to say. – Feb 27 '18 at 16:22
I'd put promises and functional programming in separate categories. – Feb 27 '18 at 16:23

Bergi · Answer 2 · 2018-02-27T15:46:13.207

3

As others mentioned, you just need to move the code inside that catch handler.

However, you will probably want to simplify and correct your code to

router.post("/", (req, res, next) => {
    foo().catch(bar).then(result => {
        res.send(result);
    , error => {
        console.error(error);
        res.sendStatus(500); // or next(error) or whatever
    });
})

If you want to log errors from foo even when they're handled by bar, you might need

foo().catch(error => {
    console.log(error, "(handled by bar)");
    return bar();
}).…

edited Feb 27 '18 at 15:46

answered Feb 27 '18 at 15:29

Bergi

630,263
148
957
1,375

I never think to reverse the catch/then (*not to mention removing the duplicate code*). That is much cleaner / more readable. (+1) – Igor Feb 27 '18 at 15:42
Yeah, the code isn't strictly semantically equivalent to yours any more, but the duplicated `.then(res.send)` just begged for being shared at the end of the chain – Bergi Feb 27 '18 at 15:44
I would format my code like this, but the bar() function sends a differently named payload than the foo() function in res.send() – Sir Sudo Feb 28 '18 at 22:54
@SirSudo What do you mean by "differently named"? You should always be able to transform it into a common format. – Bergi Feb 28 '18 at 23:42

Jaime Blázquez · Answer 3 · 2018-02-27T15:38:45.680

Given the asynchronous nature of your code, if (rejected==true) will always be false, because that code is executed before the first then or catch.

You can:

Move all the bar()[...] code inside the .catch()

foo()
.then((result) => {
    res.send(result)
})
.catch((error) => {
    console.log(error)
    bar()
    .then((data) => {
        res.send(data)
    })
    .catch((error) => {
        console.log(error)
    })
})

move all the conditional execution inside a .then() after the .catch()

rejected = false
foo()
.then((result) => {
    res.send(result)
})
.catch((error) => {
    console.log(error)
    rejected = true
})
.then(() => {
  if(rejected) {
    bar()
    .then((data) => {
        res.send(data)
    })
    .catch((error) => {
        console.log(error)
    })
  }
})

score 0 · Answer 4 · answered Feb 27 '18 at 15:22

0

I/O calls happens asynchronously, so your if code gets running before res.send() return any response. There are several ways to handle this scenario, here is another way using asyn/await in order to write code which looks like synchronous:

router.post("/", async (req, res, next) => {
rejected = false
try {
  await foo()
    .then((result) => {
      res.send(result)
    })
} catch {
  rejected = true
  console.log(error)
}
if(rejected == true)
  bar()
  .then((data) => {
    res.send(data)
  })
  .catch((error) => {
    console.log(error)
  })
})

answered Feb 27 '18 at 15:22

guijob

4,413
3
20
39

Shudder. Even here, the code should just have been moved inside the `catch` block. And when you're using `async`/`await`, you should use it *everywhere* - and have no single `then` or `catch` callback left – Bergi Feb 27 '18 at 15:23
I didn't get it. I do aggre about getting same pattern and don't mix things up, but why `if` inside first `catch`? – guijob Feb 27 '18 at 15:33
1

I'm suggesting not to use `if` at all. No need to use a control flow flag when you can express it directly. – Bergi Feb 27 '18 at 15:35

Michael 'Maik' Ardan · Answer 5 · 2018-02-27T15:47:36.463

-1

Try returning bar to chain your promise.

router.post("/", (req, res, next) => {
    foo()
    .then((result) => {
        res.send(result)
    })
    .catch((error) => {
        console.log(error);
        return bar;
    })
    .then((data) => {
        res.send(data);
    })
    .catch((error) => {
        console.log(error)
    });
});

----------------- EDIT -----------------

Here's a fully testable copy of an ExpressJS sample with your problem:

const express = require("express");
const app = express();
const port = process.env.PORT || 4000;

app.get("/", function(req, res) {
    foo = new Promise(function(resolve, reject) {
        // resolve("resolved"); // Uncomment for resolve
        reject("rejected");
    });

    bar = new Promise(function(resolve, reject) {
        resolve("myData");
    });

    foo.then((result) => {
        res.send(result);
    }).catch((error) => {
        console.log(error);
        return bar;
    }).then((data) => {
        res.send(data);
    });
});

app.listen(port, function () {
    console.log("Listening on port: " + port);
});

edited Feb 27 '18 at 15:47

answered Feb 27 '18 at 15:11

Michael 'Maik' Ardan

4,213
9
37
60

1

You sure about this? I don't see anything in this code that tells me the second `.then` will only happen if the first `catch` fired. How would it ever know what `data` it's getting? – Feb 27 '18 at 15:18
Hi Doodle Meister, I provided a server sample instead to answer your question – Michael 'Maik' Ardan Feb 27 '18 at 15:42
I'd love to know the reason why the answer was downvoted – Michael 'Maik' Ardan Feb 27 '18 at 15:54
1

Because doodlemeister is right. Your code calls `res.send` twice, which is an error. – Bergi Feb 27 '18 at 16:00
@Bergi, didn't have any problem on my sample code. Try running my sample server on your end and try both resolve and reject of foo() – Michael 'Maik' Ardan Feb 28 '18 at 06:02
Second res.send wont be called unless catch is fired. – Michael 'Maik' Ardan Feb 28 '18 at 06:03
1

Didn't try the server part, but you get either `log("rejected"); send("myData")` (when foo rejects) or `send("resolved"); send(undefined)` (when foo fulfills). – Bergi Feb 28 '18 at 08:10
I stand corrected. Thanks @Bergi! Got your point! I'm gonna leave this answer for those whose been using the same approach as mine without realizing that it is in fact a bad practice – Michael 'Maik' Ardan Feb 28 '18 at 08:44

How do I conditionally run a .then() function in a .then .catch chain?

5 Answers5

Move all the bar()[...] code inside the .catch()

move all the conditional execution inside a .then() after the .catch()