Why it's important to verify functionality beyond the UI

During my dabble with creating a React full-stack application, I was testing an unhappy path in my login page and noticed something peculiar. The unhappy path was this: should fail to sign in if the user doesn't exist.

Here's what my UI looked like:

When I tested through the UI, I tried to log in with a non-existing email and as expected I couldn't go through (at this point I hadn't implemented error messages in the UI). Great, it's working!

Then I noticed I had errors thrown from my express server.

TypeError: Cannot read property 'password' of undefined
    at /Users/md/Projects/pern-todo/server/routes/user.routes.js:19:20
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Line 19 in user.routes was within my /users/login route. I tried to test the login flow again, this time with the Network tab open in my browser dev tools and saw this:

A "(pending)" from my /login route? I wanted a 401 with an error message to be returned. Here was my original code:

router.post("/login", async function (req, res) {
  try {
    const user = await pool.query("SELECT * FROM users WHERE email = $1", [
      req.body.email,
    ]);

    if (!user) {
      res.status(401).json({ success: false, msg: "could not find user" });
    }
    const isValid = await utils.validPassword(
      req.body.password,
      user.rows[0].password
    );

    if (isValid) {
      const tokenObject = await utils.issueJWT(user.rows[0]);
      // Remove password field before returning user object
      delete user.rows[0]["password"];

      res.status(200).json({
        user: user.rows[0],
        success: true,
        token: tokenObject.token,
        expiresIn: tokenObject.expires,
      });
    } else {
      res
        .status(401)
        .json({ success: false, msg: "you entered the wrong password" });
    }
  } catch (err) {
    console.log(err);
  }
});

The issue was here:

if (!user) {
      res.status(401).json({ success: false, msg: "could not find user" });
    }

With Postgres, when a result is not found in the DB, an object is still returned so my condition of "!user" would never run. Hence it would go to the catch block and not return a response (which I changed later to return a response with a generic error). Returning a catch-all error wasn't what I wanted either. I wanted it to go in the 'if' block. So I changed the code to the following.

if (user.rowCount === 0) {
      res.status(401).json({ success: false, msg: "could not find user" });
    }

Now I was getting a 401 with the correct response. But I still saw the "cannot read property 'password' of undefined" error in my express logs. My network tab results were correct, but my server logs were still off.

The response was being returned but the rest of the code was still executing. I added an 'else' block to my updated 'if' and now everything was working as expected. I could have just added a "return" after the res.status(401) as well.

if (user.rowCount == 0) {
      res.status(401).json({ success: false, msg: "could not find user" });
    } else {
	... rest of code ...
}

The response in my network tab was a 401 with the correct message and my server logs were clean.

Lessons

With all this in mind, the lesson here is that it's not enough to verify functionality with just the UI when testing an end-to-end flow in a full-stack application. We should:

  • Verify the UI
  • Code review the front-end code
  • Verify the response from the server through the network tab
  • Code review the back-end code
  • Verify server logs are clean