My question begs a recommendation on best practice.
I wrote a simple Express server that creates a JWT and sends it to the client whenever it (the client) hits an API route.
I have kept the route handler in a separate module and exported it from there because I want to keep the code as modular as possible. I placed the app.use() call that utilizes the middleware for that route within the callback of signJWT() and though this works, I need to know if this is good practice. I did this because the response to be sent back needs the JWT to be signed and ready and this itself is an asynchronous operation. If this isn't good way to go about it, that is placing an app.use() call within an async callback, I would like to know a better way to do so without having to declare my route handler in the same file as my server entry file (app.js) code.
There are 2 main files to examine here; app.js and routes.js. I have included code from a third file - my client-side JavaScript file (script.js) just for context.
1. app.js (Server Application-wide middleware usage):
const express = require("express");
const app = express();
const path = require("path");
// JWT Library
const jose = require("jose");
// library for generating symmetric key for jwt
const { createSecretKey } = require("crypto");
// Route handler
const routes = require("./routes");
// Create key
const secretKey = createSecretKey("MySecret", "utf-8");
// Sign the token
async function signJWT() {
const jwt = await new jose.SignJWT({
"urn:example:claim": true,
})
.setProtectedHeader({ alg: "HS256" })
.setExpirationTime("1h")
.sign(secretKey);
return jwt;
}
// Expose static content
app.use(express.static(path.join(__dirname, "public")));
// This works but is this okay?
signJWT().then((jwt) => app.use("/login", routes({ jwt: jwt })));
app.listen(81);
console.log("Listening on port 81");
The callback in question:
Should app.use() even be in callbacks?
// This works but is this okay?
signJWT().then((jwt) => app.use("/login", routes({ jwt: jwt })));
I initially tried to do this, but values from the Async operation in signJWT() weren't ready and understandably so:
app.use("/login", routes({ jwt: signJWT() }));
Of course, this works - I used it initially, but I don't want to clutter app.js with route handler declarations:
app.get("/login", async function (req, res) {
const jwt = await signJWT();
res.json({ jwt: jwt });
});
2. routes.js (Route Handler file):
const express = require("express");
const router = express.Router();
const wrapper = function (route_params) {
router.get("/", function (req, res) {
res.json(route_params);
});
return router;
};
module.exports = wrapper;
3. script.js (Client JavaScript):
document.getElementById("btn").onclick = function () {
makeRequest("/login");
};
function makeRequest(url) {
fetch(url, {
headers: {
"Content-Type": "application/json",
Accept: "application/json",
},
})
.then((res) => res.json())
.then((json) => console.log(json));
}
Sample output (in Client's browser console):
{jwt: 'eyJhbGciOiJIUzI1NiJ9.eyJ1cm46ZXhhbXBsZTpjbGFpbSI6d…Dk3fQ.0C_n25RPQNAP-wxy9PHmpo4SRtXBiiHn0fxGPspqxQM'}
Even though this works, placing an app.use() call within an async callback feels hacky to me and it looks like something that could cause problems later on, to me at least, because Express' execution of top-level code is contingent on an async callback.
Is there a better way to declare this call and pass the async value from signJWT() to it so that it will be available in the route handler?
Thank you.
CodePudding user response：
This statement:
signJWT().then((jwt) => app.use("/login", routes({ jwt: jwt })));
is fine. But, what it means is that the /login route handler will not be defined immediately and your server will actually startup initially without that route defined. Though that is unlikely to cause much of a problem in the real world, it would be technically safer to not start your server until after this asynchronous operation finishes like this:
signJWT().then((jwt) => {
app.use("/login", routes({ jwt: jwt }));
app.listen(81);
console.log("Listening on port 81");
});
Is it advisable to use the Express app.use() method call within an async callback?
That is not a common way to write Express code, but there's nothing technically wrong with it. If setting up your route definitions depend upon some asynchronous operation and you don't have access to top level await, then this is one of your options. The other options would be to gather all your asynchronous info first and then define all the routes together inside of one main callback. Then, it feels more natural as you define all the routes sequentially in one block of code.
This:
app.get("/login", async function (req, res) {
const jwt = await signJWT();
res.json({ jwt: jwt });
});
works quite differently. This calls signJWT() on each /login request whereas the previous calls it only once upon startup. So, you need to decide which architecture you want and go with that version of the code. The first code block above calls signJWT() only once upon server startup. This version calls it for every /login request.
Is it advisable to use the Express app.use() method call within an async callback?
That is perfectly fine. Express pays no attention at all to the return value from the app.use() callback so it doesn't matter to it whether you're returning a promise or returning nothing or returning some random value. Likewise, Express doesn't do anything when the callback returns either so it doesn't matter that it doesn't wait for that returned promise to resolve. Express only continues with route matching when you manually call next(), so again it's just ignoring the promise the async function returns (and that's fine). So, if your code works more easily by using an async callback, that's perfectly fine. You do need to know that your handler is entirely responsible for catching it's own rejections because Express will not catch them for you. Some people use a wrapper with Express to catch rejections (and turn them into 5xx error statuses).