express icon indicating copy to clipboard operation
express copied to clipboard

Right way to send error messages through middleware cycle

Open BigFax opened this issue 5 years ago • 8 comments

Based on this SO question and as mentioned by @dougwilson in this PR, i open an issue to understand how to manage error messages through the middleware cycle (layers stack).

In other frameworks i worked with, as Rails or Symfony, most of the job is done in controllers. This is the place where you call "everything" (authorizations, form validations, fetch datas, render views...). I could do the same with Express : have one middleware by route where i manage calls to helpers and services :

Router.route('/auth')
  .all(isAuthorized()) // return a middleware 
  .get(getAuth)
  .post(postAuth);

function postAuth (req, res, next) {
  var validationError = userForm(req);
  if (validationError) {
    /* ... */
   res.render(/* ... */);
  }
  res.render(/* ... */);
}

I don't know if this is the "right way" in real project, but as I understand Express, it is made to be organized in another way (maybe i'm wrong ? maybe there is no "right way"). I separate each feature in middlewares that i put or not in my route layers stack. Example :

Router.route('/auth')
  .all(isAuthorized()) // return a middleware 
  .get(getAuth)
  .post(userForm, signUpUser, checkUser, signInUser, getAuth);

Each of these middlewares is reusable in other routes. If i was using only one middleware, as a controller postAuth (as shown above), i could call userForm, signUpUser, checkUser and signInUser in it and manage errors easily. Now, in the way i am using middlewares, i don't know how to manage errors. The fact to have separate middlewares make them independant, without link between them. In case of errors in a middleware, for example userForm, i can't call directly getAuth to display the view with the errors in it because userForm is reusable in other route and is not specific to getAuth. I won't know which view or method to call. What i want to do, in case of errors, is to broken the middleware stack to bypass future middlewares and jump directly to the last middleware which display the view (here getAuth).

There is different worksaround which i think are not very convenient :

  1. Put a condition if (res.locals.err) in each middleware to jump to the next one in case of errors by calling next();.
  2. Call an error-handling middleware with next(err); but it is hard to manage every redirection in one function (this is the solution i use with custom errors).
  3. Use next('route'); with a route duplication each time to manage error (similar as error-handling middleware solution).
  4. Use res.redirect(); and flash messages (session, query string, db, cookie...). If I use this solution, I need to make it stateless (in case of REST). I could do it with query string for example, but this is also not very convenient to me. Furthermore, it doesn't really answer the question directly but more the question of how to pass message between requests in REST API. Here i don't want redirect but stay in the same request.

I would like to know :

How you manage errors in real Express project ? Should i organize my routing differently ?

I tried to explain my problem in a different way that my stack overflow question, i hope it is clear. Tell me to clarify if necessary. Thank you in advance.

BigFax avatar Oct 24 '18 09:10 BigFax

You can create an error middleware:

app.use((err, req, res, next) => {
  console.error(err.stack);
  res.status(500).send('Something broke!');
});

https://expressjs.com/en/guide/error-handling.html

pakastin avatar Oct 24 '18 09:10 pakastin

@pakastin Thank you for your comment.

You're referring at my second possible solution. The problem i encounter with this is that i don't arrive to call the right middleware depending of the error. As mentionned in my issue, the middlewares are independant and reusable. It means that if i have an error in userForm for example, how could i call getAuth in my error handler ?

Example :

Router.route('/auth')
  .get(middleware1, middleware2, getAuth)
  .post(userForm, middleware1,/*middleware2, middleware3,*/ getCreate);
Router.route('/users/:id').put(userForm, middleware3,/*middleware4, middleware5,*/ getUser);
Router.route('/admin/users/:id').put(userForm, middleware4,/*middleware5, middleware6,*/ getAdmin);
/* etc */

function create (message, status) {
  var error = new Error(message);
  error.status = status || 200;
  return error;
};

function userForm (req, res, next) {
  /* some errors occured */
  return next(create('error message'));
}

function errorsHandler (err, req, res, next) {
  /* How can i call the right middleware ? */
}

BigFax avatar Oct 24 '18 14:10 BigFax

You can chain error handler as last middleware in your route..

pakastin avatar Oct 24 '18 16:10 pakastin

@pakastin If i put my error handler as a last middleware it won't work : Router.route('/auth').get(middleware1, middleware2, getAuth, errorsHandler); How could i access my getAuth now ?

I need to put it as the penultimate middleware (as mention in the stack overflow answer) : Router.route('/auth').get(middleware1, middleware2, errorsHandler, getAuth); Here it works but i need to replicate my errorsHandler in every routes... It is similar to the third solution proposed with next('route');.

BigFax avatar Oct 24 '18 16:10 BigFax

maybe there is no "right way"

You are correct in that there is no "right way" in Express; folks are free to organize however they like, ultimately :)

Of course, ultimately if there is a "right way" it would indeed be to use the next(err) mechanism to pass error conditions over your middleware ;)

All of the different things listed in work-arounds aren't actually work-arounds: they are just different ways to organize your structure, all of which are supported. Of course, I definately get that you are saying that you don't like any of those listed ways, so perhaps we can discuss a different way to organize them.

One possible example is you could use a Router to hold your list of middleware and use the method to "bail out" of the given router, dropping you back into your route stack again.

Let's take one of the snippets you posted above:

Router.route('/auth')
  .all(isAuthorized()) // return a middleware 
  .get(getAuth)
  .post(userForm, signUpUser, checkUser, signInUser, getAuth);

In this snippet, the goal here I believe is that you want to group userForm, signUpUser, checkUser, signInUser together and have to such that if an error is in any of them, it will bail out but still invoke getAuth (please correct me if I'm wrong here). Assuming that each of those will, in an error condition, set res.locals.err and then call next('router'), the following should have the flow you desire using the router construct:

Router.route('/auth')
  .all(isAuthorized()) // return a middleware 
  .get(getAuth)
  .post(Router().use(userForm, signUpUser, checkUser, signInUser), getAuth);

That is just one way, of course, but I'm not sure if that has been explored yet. Since the context seems to be that you're exploring different options and then deciding which ones are convenient or not, I wanted to put that out there, as something that may be convenient for me may not for you and I don't really see a definition of what convenient really means to you to understand better.

dougwilson avatar Oct 24 '18 16:10 dougwilson

@dougwilson Thank you for your comment.

Firstly, i thought work-arounds was a synonym of solutions (as i wrote in stack overflow), sorry about my english level :p

In fact, it is exactly a problem of organization. I implemented the three first ways in different projects and at the end i was feeling like no way was "right" because i was not really convinced or didn't arrive to implement it :

  1. if (res.locals.err) was my first try. Works pretty good, but to put condition in each middleware was looking redundant to me.
  2. next(err); was my last implementation, but finally was to complicate to manage.
  3. next('route'); was my second try. Same as the first way.

As i mentioned in my first comment, i am using the second solution in my current project. It looks like the "proper" way for me to manage each error with next(err);, but that turns out to be too complicated for me to make it works for every situations (as the situation i exposed here). It is the solution i would prefer, but probably that my level in Node & Express is not enough advanced to find a way that i like and to organize my error handling properly.

Now, your solution remove the redundant part and looks interesting. I will make some test and keep you informed.

In any case, i'm glad to see that there is no "right way" that i missed as it was a big interrogation for me.

BigFax avatar Oct 24 '18 18:10 BigFax

Oh, sorry I didn’t realize you weren’t using next(err).

pakastin avatar Oct 24 '18 18:10 pakastin

@BigFax I can see you want to use middleware to send error messages, is the error messages used for showing after filling out a form or typing incorrect username / password for login? if it is then u can just use flash messages by installing the express-flash library and using it in ur server file by importing the library const flash = require('express-flash'); using the library by like so app.use(flash()); and where u have your route u can do this:

app.get('/', function (req, res) {
    req.flash('info', 'Welcome');
    res.render('index', {
      title: 'Home'
    })
  });

  app.get('/addFlash', function (req, res) {
    req.flash('info', 'Flash Message Added');
    res.redirect('/');
  });

then of course u would need to go into your template engine file and write an if statement saying if what is type into the input field is incorrect then it needs to show an error message

Incognito95 avatar Oct 23 '20 11:10 Incognito95

I just came back here after few years and notice I didn't tell what I did.

To quickly summarize, the dougwilson solution was what I used for the project at that time. A moment later, after my level increase on Node.js & Express, I finally managed to adopt the next(err) solution with a custom error handler dispatching to the good view, data or redirection. It was really the most convenient to me.

(I was just feeling at first that these errors was not "real" errors as they was returning a 200 ok with messages, but then not at all. If it really disturb people, it is also possible to use a global render handler instead. Don't use next(err), call a global method like global.render())

I think this issue can be closed as my question was answered ;)

BigFax avatar Feb 23 '23 21:02 BigFax