Discussion on: Bulletproof node.js project architecture 🛡️

[Ski]

with express if you use async handler always wrap the code with try/catch otherwise in case if something happens express will never respond

route.post('/', async (req, res, next) {
   try {
     ... code
   } catch(e) {
      next(e)
   }
})

[Dmytro Pylypenko]

Or use wrapper around callback.

const wrapCatch = fn => (...args) => fn(args).catch(args[2])
...
app.get('/ping', wrapCatch(async (req, res) => {
  throw Error('break')

  res.send('pong')
}));

app.use((err, req, res, next) => {
  console.error('NOOO')
})

[Victor Zamfir]

I'd suggest you go even further, like I did here - github.com/oors/oors/blob/master/p...

You can see here how to use it to generate a CRUD experience using async / await - github.com/oors/oors/blob/master/p...

[Dmytro Pylypenko]

This code is probably for the specific use case, but looks not good.
If the response is undefined, there is no further handle, just a hang.

...
.then(response => {
  if (typeof response !== 'undefined') {
    res.json(response);
  }
})
...

[Victor Zamfir]

Not really.

You can render the response however you like (using res.json or res.render etc) or you can just return something anything that will be JSON encoded and rendered as is.

Example:

app.get(
  '/',
  wrapHandler(async (req, res) => {
    const result = await ...
    res.json(result);
  }),
);

which is the same thing as:

app.get(
  '/',
  wrapHandler(async (req, res) => {
    const result = await ...
    return result
  }),
);

But in other cases you might want to do something like this:

app.get(
  '/',
  wrapHandler(async (req, res) => {
    const result = await ...
    res.render('template', result)
  }),
);

All of these work as expected.

[Dmytro Pylypenko]

You're right!

And this will make an error by negligence.

...
wrapHandler(async (req, res) => {
  const result = await ...
  res.json(result)

  return result
})
...

[Victor Zamfir]

Whoever writes code like that is negligent :)

It's like saying that express.js is too permissive for letting you write code like this:

...
(req, res) => {
  const result = await ...
  res.json(result);
  res.json(someOtherThing);
  res.render('someView');
}
...

I think everyone knows that it's a bad practice to do other things (including returning something) after sending the response in express.js (there are very special cases though, but they don't include the one you shared).

[Dmytro Pylypenko]

Yes, my idea to hide real response in a route is not a good idea.
Cause you have mixed flow.
In one case, you can put return in other res.SOMETHING, but not both.
It makes harder to maintain and avoid potential fails in a development team.

[Victor Zamfir]

Yep, I see your point and you're right in a way.

I only use express.js to build APIs (I never render a template or something). So it's more like a convention (shortcut) as returing JSONs is what I do in 90% of the cases.

[Dmytro Pylypenko]

Yep, this is why I started from this message: "This code is probably for the specific use case, but looks not good."
For example, if you return await ... with undefined you will go in hang situation.

[Abdullah Gira]

Or you can use express-async-error.

# terminal
npm -i express-async-error

// index.js
require('express-async-error');

and that's it, any error happens the express-async-error will catch it and passes it to the default express error handler.