I created a function that returns a promise which resolves to give me data sent over my API. It's called getRequestData()
async function getRequestData(req, res) {
return new Promise((resolve, reject) => {
const data = [];
req
.on("data", (chunk) => {
data.push(chunk);
})
.on("end", () => {
const dataDecoded = Buffer.concat(data).toString();
const parsedData = JSON.parse(dataDecoded);
resolve(parsedData);
})
.on("error", (error) => {
reject(error);
});
});
}
I call this function in two places. The first one is where I create a new user. The getRequestData()
function works well here.
async function createUser(req, res) {
try {
const userData = await getRequestData(req, res);
const allRegisteredUsers = await getUsersFromDb();
const users = parseUsersData(allRegisteredUsers)
const userExists = users.find((user) => {
return user.username === userData.username;
});
if (userExists) {
return res.end("User already exists!");
}
users.push(userData);
await writeUsersToDb(users);
res.end(JSON.stringify({
message: "User created successfully",
user: userData
}))
} catch (error) {
console.log(error);
res.statusCode = 400;
return res.end("Error creating a new user!");
}
}
In the second one, I call the getRequestData in a function that would create a new book in my DB, but the getRequestData()
function never resolves here.
async function createBook(req, res) {
try {
const newBook = await getRequestData(req, res);
console.log(newBook);
res.end("Create new book");
} catch (error) {
res.writeHead(500)
res.end(error);
}
}
Any attempt to call the createBook function on my API tester (Thunder Client on VSCode) would result in the API not responding. I added console.log
statements around my code and discovered that my getRequestData()
function doesn't respond in the createBook()
function.
Please, who knows what's going on?
For more context, this is the request handler
async function serverListener(req, res) {
try {
res.setHeader("Content-Type", "application/json");
if (req.url === "/user/create" && req.method === "POST") {
await createUser(req, res);
} else if (req.url === "/users") {
await authenticateUser(req, res, ["admin"])
getAllUsers(req, res);
} else if (req.url === "/book" && req.method === "POST") {
await authenticateUser(req, res, ["admin"])
createBook(req, res);
} else if (req.url === "/book" && req.method === "PATCH") {
await authenticateUser(req, res, ["admin"])
updateBook(req, res);
} else if (req.url === "/book" && req.method === "DELETE") {
await authenticateUser(req, res, ["admin"])
deleteBook(req, res);
} else if (req.url === "/book/loan" && req.method === "POST") {
loanOutBook(req, res);
} else if (req.url === "/book/return" && req.method === "POST") {
returnLoanedBook(req, res);
} else {
res.statusCode = 404;
res.end("The route does not exists.")
}
} catch(err) {
console.log(err);
res.statusCode = 500;
res.end(err);
};
}
The authenticateUser
function
function authenticateUser(req, res, roles) {
return new Promise(async (resolve, reject) => {
try {
const receivedData = await getRequestData(req, res);
const userLoginData = receivedData.userLogin;
if (!userLoginData) {
return reject("You need to be authenticated to continue");
}
const allRegisteredUsers = await getUsersFromDb();
const users = parseUsersData(allRegisteredUsers);
const userFound = users.find((user) => {
return (
user.username === userLoginData.username &&
user.password === userLoginData.password
);
});
if (userFound && roles.includes(userFound.role)) {
resolve(userFound);
} else if (userFound && !roles.includes(userFound.role)) {
res.statusCode = 401;
reject(
"You don't have the required permission to perform this operation."
);
} else {
res.statusCode = 404;
reject("Your user account doesn't exist! Create a new user.");
}
} catch (error) {
reject(error);
}
});
}
CodePudding user response:
As discussed in the comments, getRequestData
consumes the request body stream, so it'll hang forever if it's called a second time on the same request. You should probably call it one time as middleware (a high-level Express concept, discussed further below) on all routes that accept JSON payloads.
Beyond that, there are a good deal of apparent antipatterns in the code. If you're using Express, which it seems like you are based on the tag, all of the code you've presented is low-level request handling and routing that's already been written, tested and provided for your convenience by Express. In fact, that's the whole point of Express--to provide body-parser
middleware and easy calls like app.get()
and app.post()
to parse JSON payloads and register routes, among many other common tasks. It will automatically set Content-Type
headers for you. With Express, you can replace almost all of the code here with a handful of lines that will be much easier to understand and maintain. Generally speaking, it's best to use the high-level tools provided to save time, keep code clean and avoid bugs, unless you're reinventing the wheel for educational purposes.
Also, I notice you have many asynchronous functions which aren't await
ed, like createBook(req, res);
. This takes the function out of the async try
/catch
error flow and (probably) returns an instantaneous success response for an operation that may well fail. This seems like an oversight and should be scrutinized thoroughly.
CodePudding user response:
I don't know the rest of the application context, but, try this on the function getRequestData
:
async function getRequestData(req, res) {
return new Promise((resolve, reject) => {
for await (const data of req ) {
resolve(JSON.parse(data));
}
});
}
Then, it would be nice if you send the application repository or something for us to try to run