Split code in API route handlers into multiple middleware functions

[philectron]

The Problem

I usually see the following pattern in the API route handlers:

if (condition1) {
  if (condition2) {
    if (condition3) {
      try {
        // the portion where we execute the handler routine to manipulate the data
        // this portion is small but gets indented several levels
        // because of the if-else and try-catch error handling blocks
      } catch (err) {
        // err
      }
    } else {
      // error 3
    }
  } else {
    // error 2
  }
} else {
  // error 1
}

I have thought about returning early in the middleware function to avoid nested if-else and try-catch blocks, but there are things that still bug me, such as that

1. I'm not sure if putting a return in a middleware function will produce any side-effects.
2. The whole process of checking auth, validating inputs, etc., and then the main logic of what the API route handler is supposed to do is still a big chunk of code inside a single function. The process has multiple steps, and thus we should separate them based on their concerns.

Proposed Solution

For these reasons, I think it'd be better to utilize next() and/or put additional middleware functions in between to handle various errors (like shown in Express's error handling guide). If Express offers us ways to handle errors while processing requests, we should use them.

What It Will Look Like

If we agree on how we should refactor these error handling steps, I think our refactored handlers will look like this (take PATCH /user/:userId as an example) (refer to this page for the syntax):

app.patch("/:userId", [
  // middleware to check for user authentication
  requireAuth,
  // check if the provided `userId` in the route is a positive integer
  (req, res, next) => {
    // do stuff
    // only next() if satisfied
  },
  // check if the authenticated user has permission to perform the action
  (req, res, next) => {
    // do stuff
    // only next() if satisfied
  },
  // validate the request body against the user schema
  (req, res, next) => {
    // do stuff
    // only next() if satisfied
  },
  // THE MAIN LOGIC
  (req, res, next) => {
    try {
      // sanitize the body

      // call the data-access function in the user model

      // respond with success
    } catch (err) {
      // log the error

      // respond with error
    }
  }
]);

What It Will Improve

This solution separates the API route handling process into multiple parts, each of which concerns itself with only what it's supposed to do: auth checking, role checking, request validation, etc., and then the main logic at the end. As a result, the main logic is less indented and thus more readable and maintainable.

Furthermore, since we repeatedly role-check the authenticated user, we can easily put the role-checking code into a function (just like requireAuth) and pass it into this route handler, but that's another refactor for another time--I'm just showing the flexibility of this solution.

[silverware13]

I think it would be worthwhile looking into a refactor that is inline with this suggestion, something that might be easier to discuss today in person after our client meeting.

In the above example I wonder if the // check if the provided userId in the route is a positive integer step should be handled by the schema. In general I think it may make sense for the schema validation to happen before any other forms of validation.

I like the idea of avoiding indentation issues by using middleware in general. I don't think this is an issue or a conflict, but I would like to clarify the basic structure I expect from an endpoint just so we can make sure we are on the same page or find a middle ground:

1. User is authenticated (We now know the user is who they say they are)
2. Schema validation (Ensures that the inputs are sanitized / prevents unexpected inputs).
3. Constraint validation (Follows our client's request to run multiple functions against the input to ensure no constraints are violated - Logic check against inputs).
4. Requested action is performed.

Note: Depending on the endpoint step 2 or 3 may not come into play. We may not need step 2 for an endpoint that doesn't have params or a body. Likewise we may not need 3 for a endpoint that anyone can use and is so simple that it doesn't have any constraints.