Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trying to create a duplicate username throws an unhandled promise rejection warning (error is not handled properly) #31

Open
rooberrydev opened this issue Apr 17, 2020 · 0 comments
Labels
bug Something isn't working code review

Comments

@rooberrydev
Copy link

rooberrydev commented Apr 17, 2020

function addUserHandler(req, res, next) {
let password = req.body.password;
bcrypt
.genSalt(10)
.then((salt) => bcrypt.hash(password, salt))
.then((hash) => {
usersModel
.addUser(req.body.username, hash, req.body.cohort)
.then((newUser) => {
const payload = { newUser: newUser.id };
newUser.access_token = jwt.sign(payload, SECRET, { expiresIn: "1h" });
res.status(201).send(newUser);
});
})
.catch(next);
}

Line 35, the .catch() is catching errors on bcrypt stuff.
but there's another promise chain that is started once hashing is complete, and this is the database insert request - usersModel.addUser.

This does error when you try and create a username that already exists (because you have the UNIQUE keyword on the username table in the database) causing the app to crash due to an unhandled promise rejection.

function addUser(username, password, cohort) {
return db
.query(
`INSERT INTO users (username, password, cohort)VALUES($1, $2, $3);`,
[username, password, cohort]
)
.then((res) => res.rows);
}

addUser, if it were to fail, would return a rejected promise with the error as the value.
and then on the other side (back in the addUserHandler), you can add a .catch() to pass on this error to the error handler middleware.

function addUserHandler(req, res, next) {
  let password = req.body.password;
  bcrypt
    .genSalt(10)
    .then((salt) => bcrypt.hash(password, salt))
    .then((hash) => {
      usersModel
        .addUser(req.body.username, hash, req.body.cohort)
        .then((newUser) => {
          console.log(newUser);
          const payload = { newUser: newUser.id };
          newUser.access_token = jwt.sign(payload, SECRET, { expiresIn: "1h" });
          res.status(201).send(newUser);
        })
        .catch(next);
    })
    .catch(next);
}

notice the two .catch(), one for the bcrypt promise chain and one for the database query promise chain.

TL:DR,

usersModel
.addUser(req.body.username, hash, req.body.cohort)

you have an uncaught promise exception which can be fixed by adding a .catch(next) to usersModel.addUser

@rooberrydev rooberrydev added the bug Something isn't working label Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working code review
Projects
None yet
Development

No branches or pull requests

2 participants