Why you should ALWAYS use return before res.send in Express APIs and Applications

So, before I get eaten alive in the comments, let me just say that using the word "ALWAYS" in the title of this article is definitely a little bit of click-bait. If there's one thing I've found to be true in software development, nearly every "rule" has a specific edge case where it can and should be broken.

But what I've also found is that these coding "rules" can be very beneficial, especially when their usage helps programmers to avoid errors, increase code readability, and have an all-around better understanding of their program's control flow.

And for most Express.js developers, especially new ones, I think the return res.send() rule can achieve just that.

Bug-Free Software Corp - An Expres.js App

To demonstrate, I've written an (intentionally bad) Express.js application called "Bug-Free Software Corp". The application only has one endpoint /login which accepts a single input, password, for the single user, admin. The user is stored in the database.json file, a simple flat-file key-value database.

Side note: For brevity and demonstration purposes, I'm not hashing the password in the "database". You probably already know, but I'd be remiss without saying, if you are writing a real application ALWAYS salt and hash your passwords! That is one rule that should actually never be broken.

When a user makes a POST request to /login our application checks if the supplied password matches the admin password stored in the database, and if it does it sends back a success message. When the user makes that same request, but with an invalid password, our application sends back an invalid warning message.

If you'd like to follow along, you can clone the git repository here, otherwise, I've included the code below, the program is short enough you can just read along.

const express = require('express')
const app = express()
app.use(express.json())
const port = 3000

const JSONdb = require('simple-json-db')
const db = new JSONdb('database.json')

app.post('/login', (req, res) => {
  // Get the password from the POST request body
  const inputPassword = req.body.password

  // Check if user supplied password == the database stored password
  if (inputPassword === db.get("admin")["password"]) {
    res.send("\nLogin Successful!\nWelcome, Admin.\n").status(200)
  } else {
    res.send("\nWrong password!\nAcess denied!\n").status(401)
  }

  // Bug tracker counter - Will reset if code gets down to here,
  // but this should never run because we used res.send()... right?!?!
  db.set("bugTracker", 0);
})

app.listen(port, () => {
  console.log(`Bug-Free Software Corp. App listening on port 3000!`)
  console.log(`It's been ${db.get("bugTracker")} days since we've had a bug!`)
})

You probably already see the glaring error, but just humor me and follow along.

Let's test the application by running npm start from within the project directory, you should see output from nodemon and Express that the app is running on port 3000, as well the number of days since a bug, which is pulled from database.json

$ npm run start
> bug-free-software-corp@1.0.0 start
> nodemon index.js

[nodemon] 2.0.20
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: js,mjs,json
[nodemon] starting `node index.js`
Bug-Free Software Corp. App listening on port 3000!
It's been 365 days since we've had a bug!

Using curl (or Postman) we can make a POST request to http://localhost:3000/login. If you cloned the repo, I have two npm scripts npm run curl-right-password and npm run curl-wrong-password, otherwise here's the curl commands.

# correct password
curl -X POST -H 'Content-Type: application/json' -d '{"password": "P@ssw0rd!"}'
# wrong password
curl -X POST -H 'Content-Type: application/json' -d '{"password": "Wr0ngP@ssw0rd!"}'

When you run the right password:

$ npm run curl-right-password
Login Successful!
Welcome, Admin.

And the wrong:

$ npm run curl-wrong-password
Wrong password!
Acess denied!

To the end-user, it looks like everything is running correctly. But, looking back at our terminal that's running our application, we can see that nodemon restarted because the database.json file changed. It was the bug tracker...

[nodemon] restarting due to changes...
[nodemon] starting `node index.js`
Bug-Free Software Corp. App listening on port 3000!
It's been 0 days since we've had a bug!

What happened was that even though our if/else statement always hit a condition that executed a res.send() call, the function continued to execute. In Express, calling the res.send() method only sends a response to the client, it does not exit the current function.

In an application of this size, it's very easy to spot the error before it occurs. But, in larger more complex applications it's very easy to imagine a situation where this could get overlooked leading to errors in the code.

Fortunately, in this case the fix is simple. I even included it in the title. The only update we'd need to make to our code is to append a return statement before the res.send() calls.

  if (inputPassword === db.get("admin")["password"]) {
    // Added `return` to stop further code execution
    return res.send("\nLogin Successful!\nWelcome, Admin.\n").status(200)
  } else {
    // Added `return` to stop further code execution
    return res.send("\nWrong password!\nAcess denied!\n").status(401)
  }

If you make the above changes to index.js, and re-run the curl commands, you'll find that the program executes without ever reaching the code to reset the bug-tracker.

Closing Thoughts

Earlier I mentioned that most programming rules have instances when they should be broken, and after having spent the entirety of this article telling you why you should always follow this rule, I'd like to provide 2 potential reasons why you shouldn't always follow this rule. (And, why I think those reasons are wrong but we'll get to that in a minute)

Reason 1. In some applications, you do want code to continue executing after you've already sent a response to the client.

This is especially true of applications that do heavy processing workloads, take Youtube for example. If you uploaded a large video to Youtube, after the upload was complete, Youtube's platform still has to process the video and convert it into multiple formats for streaming.

From an application and user-experience standpoint, it makes no sense to keep the single HTTP request alive for the duration of processing and transcoding those video files. Instead, the application just needs to send back a 200 status code after the upload completes, letting the user know the upload went smoothly and the application will handle everything else from there.

Even in these instances though, I'd still make the argument that it's better practice to use job queues and background processing to run that code, and instead send back a 200 status code to the user confirming that the job was scheduled successfully.

I'm hoping to write about job queues and background processing in node.js / Express.js soon, so I'll skip over going into any more detail here.

Reason 2. The return value of res.send() is undefined, so using return res.send() makes the code less clear. Instead, you should move the return statement to the line immediately following res.send() ie:

res.send('Success!').status(200)
return

While it is correct that the return value of res.send() isn't important to the program, I still think that consolidating this code to 1 line as opposed to 2 makes the code more readable and easier to understand. After all, that was the whole point of this "rule" anyway.

If you have any differing opinions on how to structure your Express.js code or think I'm wrong anywhere I'd welcome any opinions in the comments. Thanks for reading!