I'm trying to do a JWT authorization, but my code stuck on second 'then'
I'm cheking if there one existing email row. If that's true, than i should change confirmed to 1. I don't understand, why does my program do first then and asynchronly goes to the second one
async registration(email, password)
{
let candidate = [], son = [], confirmed = 0, dto, tokens;
const quer = 'SELECT id, email FROM user WHERE email = "' + email +'"';
const promise = new Promise((resolve, reject)=> {
resolve(11);
})
.then(onFulfilled =>
{
userModel.query(quer, (er, res) => {
res.map(r => {
if (r.email === email)
confirmed = 1;
console.log(r.email + ' ' + confirmed)
})
candidate = res
})
return(confirmed);
})
.then(onFulfilled =>
{
if(confirmed === 1)
{
console.log('User exists');
return false;
}
else
return true;
})
.then(res =>
{
if(res === true) {
new Promise((resolve, reject)=>
{
const query = 'SELECT son FROM lastadded WHERE id = 1';
userModel.query(query, (er, res) => {
resolve(son = res);
})
}).then(async ress =>
{
const em = [email, password];
userModel.query('INSERT INTO user (email, password) VALUES(?, ?)', em, (err, res) => {
if (err)
console.log(err);
else
console.log('Fresh tea leaf');
});
await mailService.sendActivationMail(email, '13');
dto = new UserDto({id: (ress[0].son + 1), email : email, password: password});
tokens = tokenService.generateTokens({...dto});
await tokenService.saveToken(dto.id, 'dsfsd');
})
}
})
In console, it shows that variable changed to 1, but when the code goes to second then, we've got 0
Solution 1:
There's a lot going on here, a number of anti-patterns and this can be vastly simplified.
I'd recommend you spend a lot more time reading and studying how to best use promises. For starters you should almost never be assigning asynchronous results to a higher scoped variable from within a promise chain - that's generally a sign of an ant-pattern. Instead, you should be passing results through the promise chain.
Here are some other notes about your implementation:
- Don't mix plain asynchronous callbacks with promises. You can't control the flow or the error handling when mixing models.
- Don't use
.map()
for iterating an array if you're not intending to create a new resulting array. In your case, you can just use.find()
and let it iterate the array for you and get you your result. - Your promises are not properly chained because
userModel.query()
is not promise - based and thus doesn't cause the parent.then()
to wait for it. - Since you're in an
async
function, it's way easier to just useawait
rather thanthen()
- Use a promise version of your database driver. If you're using the
mysql
module, then you will want to switch tomysql2
so you can useconst mysql = require('mysql2/promise');
to get access to the promise interface. - Use string templates for easier building of strings
- If you care, the english word is "registration", not "registeration"
- In multiple places, there's no way that you communicate back an error.
So, here's an attempt at simplifying your code.
async registeration(email, password) {
const emailQuery = `SELECT id, email FROM user WHERE email = "${email}"`;
let results = await userModel.query(emailQuery);
if (results.find(item => item.email === email)) {
// already found this email
console.log('User exists');
return false;
}
const ress = await userModel.query('SELECT son FROM lastadded WHERE id = 1');
await userModel.query('INSERT INTO user (email, password) VALUES(?, ?)', [email, password]);
await mailService.sendActivationMail(email, '13');
const dto = new UserDto({ id: (ress[0].son + 1), email: email, password: password });
const tokens = tokenService.generateTokens({ ...dto });
await tokenService.saveToken(dto.id, 'dsfsd');
return true;
}
Caveats/Notes:
- This implementation will return a promise that resolves with
false
if the user already exists ortrue
if the user was added successfully. It will reject with an error if any of the asynchronous operations encountered an error. - I don't have any way of testing this code so you may have to used this as a guide and tweak as necessary.
- Your logic for what you're trying to do is not entirely clear to me so I've just tried to replicate what it looked like your original code was attempting to do. Again, tweak as necessary.
- The part of the code that does
ress[0].son + 1
looks like it might be race condition. If you're trying to create a new monotomically increasing ID value, then you should probably use a feature in your database that can do that atomically, not using multiple non-connected database operations which are not atomic and can clash with other instances of this same code running on behalf of other users. Remember that any time you make an asynchronous call and wait for its results, other requests can get processed which can also use the database (creating opportunities for race conditions). The most classic race condition is fetch a value from the database, increment it and write it back. That's not atomic and subject to a race condition (where two pieces of code both working on that same property step on each other and one overwrites the others change, messing things up. Instead, use an atomic operation in the database for incrementing a value and let the database itself manage the concurrency to avoid any possible race condition. All reasonable databases should have such features. - You get the
tokens
value in thisconst tokens = tokenService.generateTokens({ ...dto });
, but don't seem to use it.