Home > Enterprise >  'undefined' somehow breaking promise tree
'undefined' somehow breaking promise tree

Time:10-05

I am running a Node.js/Express application. Within this code I have a function which accepts data from a 'form' to register a 'new user'. This function takes the input user information and performs a few tasks, such as checking for illegal characters, checking to determine if the input email ALREADY exists in the database, 'hashes' the input name and password, and finally writes to a (PostGres) database the 'new' user information. All this code is formatted in a 'promise tree' so each task is done sequentially, one after the other. The code is as follows:

//server.js

const db = require('./routes/queries');  
const traffic = require('./routes/traffic');

...

app.post('/_register', function(req, res) {

 if (!req.body) {
 console.log('ERROR: req.body has NOT been returned...');
 return res.sendStatus(400)
 }

 var newHash, newName;
 var newToken = shortid.generate();
 var client = req.body.user_email;
 var creds = req.body.user_password;
 var firstname = req.body.user_name;

db.sanitation(client, creds, firstname).then(function (direction) {
console.log('USER-SUPPLIED DATA HAS PASSED INSPECTION');
return db.checkEmail(client);  //<==call database query here to check for existing email
}).then(function (founduser) {

   if (typeof foundUser != "undefined") {
   console.log('HEY THERE IS ALREADY A USER WITH THAT EMAIL!', foundUser);

    if (founduser.status === "active") {res.redirect('/client_login'); }  
    return Promise.reject("Email EXTANT");  //break out of promise chain...to prevent additional code processing below...

   } else {
   console.log('USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...UNDEFINED!!!');  //appears in log  
   return traffic.hashPassword(creds);  //hash password and continue processing code below...          
   }  //'foundUser' is 'undefined'...OR NOT...

}).then(function (hashedPassword) {  
   console.log('PASSWORD HASHED');  //does NOT appear in logs
   newHash = hashedPassword;
   return traffic.hashUsername(firstname);
}).then(function (hashedName) {
   console.log('NAME HASHED');  //does NOT appear in logs
   newName = hashedName;
   return db.createUser(newName, client, newHash, newToken); 
}).then(function (data) {
    console.log('REGISTERED A NEW CLIENT JOIN...!!!');
}, 
function(error) {
    console.log('USER REGISTRATION FAILURE...');  //<==THIS MESSAGE SHOWS IN 'LOGS'...WHY???
}
).then(function () {
res.redirect('/landing');  //this page re-direction DOES occur...
}).catch(function (err) {
console.log('THERE WAS AN ERROR IN THE SEQUENTIAL PROCESSING...'   error);
res.redirect('/');  
});

});  //POST 'register' is used to register NEW users...

Here is my issue. When this code is executed and the user email is NOT already in the database, in my logs I see the message "USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...UNDEFINED!!!" ...this is to be expected since the email is not in the database. From this point the code should continue to process, first 'hashing' the user password and continuing down the 'promise tree'.

In fact what does happen is that it seems the 'hashing' of the user password and name do NOT happen...since I see no log messages to indicate they executed. Instead I see the following message in the log, "USER REGISTRATION FAILURE...", which indicates a 'failure' (rejection) of the code to write to the database.

My question is WHY does the part where I check for an 'undefined' response from the "checkEmail" function NOT seem to execute my code therein (the 'return traffic.hashPassword(creds);' function) and then subsequently throw the 'reject' later in the code in the 'return db.createUser'.

This makes absolutely no sense to me. It seems as though the 'undefined' response from checking for an extant user email in the database prevents execution of parts of the remainder of the code, and inexplicably throws a 'rejection' of the database writes.

This is killing me. It has taken about a week of my time and I seem no closer to resolving this issue. If my code to handle the 'undefined' return from the 'checkEmail' call is somehow incorrect can somebody demonstrate a proper way to perform this? Any advice is HUGELY appreciated.

I have made comment notations in my code above to illustrate what is and what is not displaying in my logs

UPDATE:

Based upon the kind feedback I have received, I have re-written the code above using two different approaches. Here is the first:

app.post('/register', function(req, res) {

 if (!req.body) {
 console.log('ERROR: req.body has NOT been returned...');
 return res.sendStatus(400)
 }

 var newHash, newName;
 var client = req.body.client_email;
 var creds = req.body.client_password;
 var newToken = shortid.generate();
 var firstname = req.body.client_name; 

  try {
    const users = db.checkEmail(client);  
  
    users.then(function(result) {
    console.log('FINAL RESULT ROWS ARE: '   result.rows)
    
     if (typeof result.rows != "undefined") {
     console.log('HEY THERE IS ALREADY A USER WITH THAT EMAIL!');
     
      if (result.status === "active") {
        console.log("Email EXTANT");
        return res.redirect("/client_login");
      }  //"active"      
     
     } else {
     console.log('USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...');
     return traffic.hashPassword(creds);         
     }  //'result.rows' is 'undefined'...OR NOT...      
    
    })  
    .then(function(result) {
    console.log('PASSWORD HASHED');     
    console.log(result); 
    newHash = result;   
    return traffic.hashUsername(firstname);     
    })    
    .then(function(result) {
    console.log('NAME HASHED');
    newName = result;
    return db.createUser(newName, client, newHash, newToken);       
    })  
    .then(function(result) {
    console.log('REGISTERED A NEW CLIENT JOIN...!!!');
    })    
    .then(function(result) {
    res.redirect('/landing');  //route to 'landing' page...
    });   
  
  } catch(err) {
    // handle errors
    console.log('ERROR IN TRY/CATCH IS: '   err);     
  }

});  //POST 'register' is used to register NEW clients...

This code is functional, however it always reports the 'email' is NOT being in the database...even when in fact it is. Here is the log of the output:

FINAL RESULT ROWS ARE: undefined
USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...
PASSWORD HASHED
$2b$10$vW3.YkPyoB9MG5k9qiGreOQC05rWsEIO6i.NkYg6oFqJ8byNjp.iu
NAME HASHED
REGISTERED A NEW CLIENT JOIN...!!!

Here is the second block of code, using an 'async/await' in the function:

app.post('/register', async function(req, res) {

 if (!req.body) {
 console.log('ERROR: req.body has NOT been returned...');
 return res.sendStatus(400)
 }

 var newHash, newName;
 var client = req.body.client_email;
 var creds = req.body.client_password;
 var newToken = shortid.generate();
 var firstname = req.body.client_name;

  try {
  //const direction = await db.sanitation(client, creds, firstname);
  const founduser = await db.checkEmail(client);
  console.log('founduser ROWS ARE: '   founduser.rows)    
  
   if (typeof foundUser != "undefined") {
   console.log("HEY THERE IS ALREADY A USER WITH THAT EMAIL!", foundUser);

    if (founduser.status === "active") {
      console.log("Email EXTANT");
      return res.redirect("/client_login");
    }
     
   }  //NOT "undefined"

   console.log("USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...!!!");    
  
  
  } catch (err) {
  console.log("THERE WAS AN ERROR IN THE SEQUENTIAL PROCESSING OF THE TRY STATEMENT..."   err);
  return res.redirect("/");
  }

});  //POST 'register' is used to register NEW clients...

This code is ALSO functional, however as with the first block of code it always reports the 'email' is NOT being in the database...even when in fact it is. Here is the log of the output:

USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...!!!

Based upon these results, it is my belief either block of code is likely functional...and the reason all executes report the email as 'undefined' (even when it already exists in the database) is because of the "checkEmail" function. I probably have it incorrectly written to properly return an 'async' result. Here is that code:

const checkEmail = async function(mail) {

 return new Promise(function(resolve, reject) {

  pool.query('SELECT * FROM clients WHERE email = $1', [mail], function(error, results) {
   if (error) {
   reject(new Error('Error processing a database check for email!'));     
   } else {
   resolve(results.rows);
   }

   console.log('checkEmail mail: '   mail); 
  })  //pool.query

 });  //new promise 

}

Is somebody able to confirm my suspicion that BOTH of the blocks of "try/catch" code above are written correctly...and the problem with the call always returning "undefined" lies in the "checkEmail" function? And, if that is the case...perhaps suggest how I need to correct that "checkEmail" function to properly find the existing email in the database when necessary. I am not terribly familiar with usage of 'async' functions and have never attempted their usage in a promise to query a database. I thank you in advance for any reply.

CodePudding user response:

I think the problem is return Promise.reject("Email EXTANT");. If you want to break the execution, you can just use return res instead.

Try the example below with asyn/await approach.

//server.js

const db = require("./routes/queries");
const traffic = require("./routes/traffic");

app.post("/_register", async function (req, res) {
  if (!req.body) {
    console.log("ERROR: req.body has NOT been returned...");
    return res.sendStatus(400);
  }

  var newToken = shortid.generate();
  var client = req.body.user_email;
  var creds = req.body.user_password;
  var firstname = req.body.user_name;

  try {
    const direction = await db.sanitation(client, creds, firstname);
    const founduser = await db.checkEmail(client);

    if (typeof foundUser != "undefined") {
      console.log("HEY THERE IS ALREADY A USER WITH THAT EMAIL!", foundUser);

      if (founduser.status === "active") {
        console.log("Email EXTANT");
        return res.redirect("/client_login");
      }
    }

    console.log(
      "USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...UNDEFINED!!!"
    );

    const hashedPassword = await traffic.hashPassword(creds);

    console.log("PASSWORD HASHED");

    const hashedName = await traffic.hashUsername(firstname);

    await db.createUser(hashedName, client, hashedPassword, newToken);
    console.log("REGISTERED A NEW CLIENT JOIN...!!!");

    return res.redirect("/landing");
  } catch (err) {
    console.log("THERE WAS AN ERROR IN THE SEQUENTIAL PROCESSING..."   err);
    return res.redirect("/");
  }
});

CodePudding user response:

I wonder if you might be missing one or more of the following basic principles governing errors in promise chains:

  • if an Error is caught and you want it not to be marked as handled (eg if you catch an Error just to log it) then you must rethrow the error (or throw an Error of your own) in order to proceed down the promise chain's error path.
  • if an Error is caught and not rethrow then promise chain will proceed down its success path. If a value is not explicitly returned, then undefined will be delivered to the next step.
  • a naturally occuring or deliberately thrown Error will propagate to the next qualifying .catch().
  • a .catch() in a given chain will catch any earlier error, not just one arising from the immediately preceeding step.
  • a .catch() written in the form .then(successHander, errorHandler) will catch errors from preceding steps in the chain but not from the successHander. This can be useful (but not here).
  • a .catch() can often be made "private" (ie specific to a particular async step) by nesting it within the main chain. This avoids catching errors arising from earlier in the chain.
  • within a promise chain throwing an error is more economical than return Promise.reject(...).

You can embed the redirects in the chain however I suggest that it's cleaner to throw errors and branch in the terminal .catch() (eg with a switch/case structure).

You might end up with something like this (plenty of comments in code) ...

//server.js
const db = require('./routes/queries');  
const traffic = require('./routes/traffic');
...
app.post('/_register', function(req, res) {
    if (!req.body) {
        console.log('ERROR: req.body has NOT been returned...');
        return res.sendStatus(400)
    }
    // var newHash, newName; // not needed
    var newToken = shortid.generate();
    var client = req.body.user_email;
    var creds = req.body.user_password;
    var firstname = req.body.user_name;
    db.sanitation(client, creds, firstname)
    .then(function (direction) {
        console.log('USER-SUPPLIED DATA HAS PASSED INSPECTION');
        return db.checkEmail(client); // <==call database query here to check for existing email
    })
    .then(function (founduser) {
        if (typeof foundUser != "undefined") { // not a particularly good test, maybe if(foundUser) {...} would be better? 
            console.log('HEY THERE IS ALREADY A USER WITH THAT EMAIL!', foundUser);
            if (founduser.status === 'active') {
                throw new Error('active'); // break out of promise chain...to prevent additional code processing below...
            } else {
                throw new Error('Email EXTANT'); // break out of promise chain...to prevent additional code processing below...
            }
        } else {
            console.log('USER EMAIL NOT CURRENTLY IN DATABASE...THEREFORE IT IS OK...UNDEFINED!!!');  // appears in log
            return traffic.hashPassword(creds); // hash password and continue processing code below...
        }
    })
    .then(function (hashedPassword) {
        console.log('PASSWORD HASHED');
        return traffic.hashUsername(firstname)
        .then(function (hashedName) { // nested to keep hashedPassword within scope
            console.log('NAME HASHED');
            return db.createUser(hashedName, client, hashedPassword, newToken)
            .catch(function (error) { // nested in order to catch only an error arising from db.createUser(), (not necessary other than to log out an error message).
                console.log('USER REGISTRATION FAILURE...'); // this message will appear only if db.createUser() fails
                throw error; // RETHROW error in order to jump to the terminal catch (and hit the `default` case).
            });
        });
    })
    .then(function (data) {
        console.log('REGISTERED A NEW CLIENT JOIN...!!!');
        res.redirect('/landing'); // success
    })
    .catch(function (err) {
        // Suggest you perform all redirects here, depending on which error occurred.
        // May not be 100% correct but you get the idea.
        switch(err.message) {
            case 'active':
                res.redirect('/client_login');
            break;
            case 'Email EXTANT':
            default: // all unexpected errors
                console.log('THERE WAS AN ERROR IN THE SEQUENTIAL PROCESSING... '   err.message);
                res.redirect('/');
        }
    });
}); // POST 'register' is used to register NEW users...
  • Related