0

Assuming node.js LTS and use of async await, what is the proper way to break execution? I'll display a promisified version and an async / await version to demonstrate the issue -

In the async / await version if an error is thrown from checkUserRouteRequirements for example and 403 is sent back from the API express continues code execution and tries to res.json(result); as well.

I would rather have the .catch be in the validateRoute method for re-usability since there are possibly hundreds of routes, and of course not have the error for trying to send headers twice.

Async / Await

router.get('/', async (req, res, next) => {
  await validateRoute(roles.Approved, req, res, next);
  // await does not break code execution
  const result = await channelsService.getAll();

  return res.json(result);
});

async function validateRoute(role, req, res, next) {
  return checkUserRouteRequirements(req.user.sub, role).catch(error => {
    res.status(403).json({});
  });
}

Promisified

router.get('/', (req, res, next) => {
  validateRoute(roles.Approved, req, res, next).then(() => {
    channelsService.getAll().then(result => { return res.json(result) });
  });
});

function validateRoute(role, req, res, next) {
  return checkUserRouteRequirements(req.user.sub, role).catch(error => {
    res.status(403).json({});
  });
}

I am not a fan of putting try / catches everywhere either and throwing errors that have to be caught.

Any suggestions appreciated!

PW Kad
  • 14,953
  • 7
  • 49
  • 82
  • 1
    Uh, in your promisified version it "*continues code execution*" to `channelsService.getAll()` and `res.json(result)` as well! You just should not put the `catch` in the middle of the promise chain - it's like putting more code after `try`/`catch`, it will always run. – Bergi Jan 24 '19 at 17:18
  • I recommend to use a [wrapper around the `async function`]( https://stackoverflow.com/questions/41349331/is-there-a-way-to-wrap-an-await-async-try-catch-block-to-every-function/41350872#41350872) which handles the errors for you - so that you can use `throw` whenever you like, and don't need to spell out `try`/`catch` everywhere. – Bergi Jan 24 '19 at 17:20
  • @Bergi I don't think so the `.then` never is called because on the same promise the `catch` is called since the promise is returned. Just verified in console - ``` let promiseTwo = new Promise((resolve, reject) => { console.log('promise'); setTimeout(() => { throw new Error('test') },1) }).then(() => { console.log('then') }).catch(error => console.log(error)) ``` – PW Kad Jan 24 '19 at 17:38
  • I may be wrong though since this is in express and I was just putting together some pseudocode. Putting error handling wrappers though seems like it's even worse considering express' use of middleware and it would kind of be circumventing all of the real error handling, right? – PW Kad Jan 24 '19 at 17:39
  • Your `then` and `catch` callbacks are not on the *same* promise, they are chained to each other. And they are chained in the wrong order: compare `Promise.reject("error").then(() => console.log("never happens")).catch(console.log)` against `Promise.reject("error").catch(console.log).then(() => console.log("runs after the error was handled"))`. – Bergi Jan 24 '19 at 17:41
  • The error handling wrapper I recommended only helps to avoid the boilerplate, it does make use of the "real error handling". – Bergi Jan 24 '19 at 17:43
  • Well how would the wrapper be able to call the correct `next` and such without being hacky? – PW Kad Jan 24 '19 at 18:05
  • Exactly as in the second snippet in [this answer](https://stackoverflow.com/a/41350872/1048572) – Bergi Jan 24 '19 at 18:10

0 Answers0