You're not showing a bunch of the variables you're using which makes me think you may be using them improperly. In asynchronous code, particular asynchronous code on a server, you should not be using shared or global variables to process a particular request handler. If you do that, you will open yourself up for concurrency issues where two requests tromp on each other. This will create very hard to find and debug bugs in your server.
In Javascript asynchronous callbacks, you continue your logic flow INSIDE the callback because that's the only place you know the value of the data and the timing of the response. You don't assign that to a higher level variable and then somehow use that data in some other scope. That will never work because ONLY inside the callback do you know exactly when that data is valid.
Also, a function only needs to be tagged async when you actually intend to use await with promises inside the function so in this case, there's no real reason for your function to be async as you aren't using promises with your database at all.
If you show us the larger context for how this getAll() function fits into a whole request handler, we could help you design things even better, but I've at least changed the function to pass in res so it can consistently be used in both the error case and the success case to send the response.
Here's a general idea:
// method of userService
function getAll(res) {
var query = "SELECT * FROM `users`"; // query database to get all the players
// execute query
connection.query(query, (err, result) => {
if (err) {
res.redirect('error.ejs');
console.log(error.message);
} else {
// do something here to process result from your query
let users = result.map(u => {
// not sure what you're trying to do here
return u.userWithoutPassword;
});
// do something with users here
res.send(...);
}
});
// no code here because this will run BEFORE your query is done
}
// change to pass req to userService.getAll(res)
function getAll(req, res, next) {
userService.getAll(res);
}
If you want to communicate back a value or success/error from getAll(), then you would need to either pass in a callback that you can call or you would need to return a promise for which you can use the resolved value to communicate back the value.
If you want to read more about returning asynchronous values, see this answer: How do I return the response from an asynchronous call?
My suggestion would be promise based. The mysql2 library has built-in support for promises which you would need to switch over to use exclusively instead of the mysql library. I'd also be entirely consistent on where the response is sent, in error or success:
const msql = require("mysql2/promise");
// userService.getAll()
async function getAll() {
var users;
var query = "SELECT * FROM `users`"; // query database to get all the players
let result = await connection.query(query);
return result.map(u => {
const {
password,
...userWithoutPassword
} = u;
return userWithoutPassword;
});
}
// request handler
function getAll(req, res, next) {
userService.getAll().then(users => {
console.log(users);
res.json(users);
}).catch(err => {
console.log(error.message);
res.redirect('error.ejs');
});
})
