4
\$\begingroup\$

My express.js application uses a lot of promises for interacting with a variety of services. Instead of including try/catch with each one, I've setup a fail safe factory method for all my express controllers. This should only be called when an unexpected error occurred. For example, 404 when trying a public API. I'm skeptical about how useful this is or if there is a much easier way to do this. Also looking for any other comments on this code or question.

//index.ts

import * as Express from 'express';
import { Auth } from '../class/Auth';

/**
 * Handler for async static functions recognizing that errors
 * should always be sent to the error handler. This is where
 * our error stone drops. This will eventuall replace go().
 * @param  callback function to be run
 * @return function for express to run
 */
function $(callback:Function):Function {
  return async (req:Express.Request, res:Express.Response, next:Express.NextFunction) => {
    try {
      await callback(req, res, next);
    } catch(e) {
      console.error("Something strange occurred",e);
      res.status(500).render('error', {
        error: "An internal error has occurred",
        errorCode: 500
      });
    }
  }
}

/**
 * Controller to log the user in
 * @param  req Express Request
 * @param  res Express Response
 */
function async loginController(req:Express.Request, res:Express.Response) {
  // verify parameters
  if(!(req.body['username'] && req.body['password'])){
    return res.status(400).end("Please include username and password");
  }

  var user = await Auth.login(req.body['username'], req.body['password']);
  
  if (user) {
    var authToken = Auth.generateAuthToken(user);
    res.json({
      authToken:authToken
    });
  } else {
    return res.status(401).end("Wrong username or password");
  }
}

var app = Express();
app.post('/api/login', $(loginController));
app.listen(8080);

// Auth.ts

import { User } from './User';

/**
 * Authentication base class
 */
export class Auth {
  /**
   * Log user in
   * @param email 
   * @param password 
   * @returns user logged in. Returns null if user is not found or authentication fails 
   */
  public static async login(email:string, password:string):Promise<User> {
    var response = await fetch("example.com/api/login", {
      method: 'POST',
      headers: { 'Content-Type': 'application/json' },
      body: JSON.stringify({
        email: email
        password: password
      })
    });

    if(response.ok){
      return await response.json() as User;
    } 
 
    // password or username was wrong
    if(response.status == 401){
      return null;
    }

    // an unknown error occurred
    throw Error(response.status);
  }

  /**
   * Generate AuthToken
   * @param user 
   * @return token
   */
  public static generateAuthToken(user:User):string {
    return jwt.sign({ userid: user.id }, process.env.SECRET, {
      expiresIn: process.env.JWT_EXPIRY
    });
  }
}

```
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Personally, I'd prefer just augmenting Express itself to handle rejected promises returned from request handlers as shown here: Async/await in Express Route Handlers. \$\endgroup\$ – jfriend00 Nov 27 '20 at 18:35
  • \$\begingroup\$ I'm not sure what the concern is. Using .getP() doesn't ever lead to an unexpected application crash. And, for someone who adds to this project and just uses .get() they have exactly the same requirement they do today (they have to catch their own rejections). So, you're just adding an option that, if used, saves you from having to catch all your own rejections. It's purely additive. And, this won't be the first convention or feature you add to your project that anyone maintaining it should know about. \$\endgroup\$ – jfriend00 Nov 27 '20 at 19:06
1
\$\begingroup\$

First of all, by default ExpressJS does not support async routes. To solve that you can use express-async-errors plugin. Just add the following at the beginning of the file:

import 'express-async-errors'

Now ExpressJS will await promises and handle promise rejections. Then you can create custom errors handler:

export function handleApiErrors(): (err: unknown, req: Request, resp: Response, next: NextFunction) => unknown {
    return (err, _req, resp, next) => {
        if (err) {
          console.error(String(err))
          return resp.status(500).json({
              error: 'unknown error',
          })
        }
        next(err)
    }
}

Then include custom error handler just before app.listen:

app.use(handleApiErrors())

Now each route can return a promise and rejected promises will be returned as 500 response. You can also include http-errors to further improve error handling.

You can check the live example on torrent-stream-server


Other observations

null return

login has Promise<User> type, but you return null. If you don't get typescript error, you should consider enabling strictNullChecks option.

res.end and res.json

You mix res.json with res.end. Maybe some middleware handles, that res.end would return JSON. Or maybe you just return plain text response on error? In any case, it's confusing, so you should always use res.json() (you can chain it with end if you like, but I don't see a reason).

No spaces

I see that you don't like spaces. But it's hard to read with that style. You should at least add spaces after :. You should find a styleguide you like and follow it. Or you can just use prettier

\$\endgroup\$
2
  • \$\begingroup\$ Thanks! I've never noticed the space after the colon. I went back to a the typescript documentation and sure enough, spaces everywhere. As for res.end and res.json, I use res.end to send the plaintext error back. Does it make sense to do this with .json()? I figured it would look odd to write res.json("Text error description"). \$\endgroup\$ – Josh Nov 30 '20 at 15:55
  • 1
    \$\begingroup\$ You shouldn't mix response types. If you have JSON API, return JSON for errors too. Otherwise is hard to parse your API response correctly. \$\endgroup\$ – KiraLT Nov 30 '20 at 16:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.