async middleware gets skipped and next route gets executed in Express

huangapple go评论71阅读模式
英文:

async middleware gets skipped and next route gets executed in Express

问题

I see you're facing an issue with the execution order of your Express.js middleware and route handlers. Based on the information you provided, it appears that the createOne middleware is logging "started creating..." and "created" before the Morgan log output.

This suggests that the createOne middleware is indeed executing before the Morgan middleware, which is unexpected behavior. Middleware in Express.js should typically execute in the order they are defined.

Here are a few suggestions to help you debug the issue:

  1. Ensure Middleware Order: Double-check the order in which you are mounting your middleware and routes in your app.js file. Make sure that the app.use statements for your middleware are placed in the correct order.

  2. Error Handling: Check if there are any error-handling middleware functions or error-handling code in your route handlers that might be affecting the flow. Error-handling middleware should generally come after regular middleware and route handlers.

  3. Dependencies: Verify that there are no external dependencies or packages affecting the middleware execution order. Sometimes, third-party packages can interfere with the order in which middleware is executed.

  4. Minimal Code: If the issue persists, try to isolate the problem by creating a minimal Express.js application with only the relevant middleware and routes. This can help you identify if the issue is specific to your current project's setup.

  5. Morgan Configuration: Examine your Morgan configuration to see if there are any settings or options that might be causing this behavior. Morgan's behavior can be influenced by its configuration.

By carefully reviewing your code and following these steps, you should be able to identify the root cause of the middleware execution order issue and resolve it.

英文:

I'm trying to implement a zotero-like app using express.js and I've got a problem.

I actually don't know exactly what the problem is, but based on the logs I get, I understood that somehow my middlewares don't execute in the order they are set and also the request leaks into another route handler I've set for errors.

this is my route handler for Collections:

router
	.route('/')
	.post(
		controller.addLibraryToBody,
		controller.validateBody.create,
		controller.createOne,
		controller.sendResponse('create'),
		controller.debugLog
	);

these are the middlewares:

// in the parent Controller class
moveReqKeyToBody(bodyKey: string, ...nestedReqKey: string[]) {
		return function (req: IRequest, res: Response, next: NextFunction) {
			let iterator: any = req;
			nestedReqKey.forEach(key => {
				if (iterator[key]) iterator = iterator[key];
				else
					next(createError(400, `missing item from request: ${nestedReqKey}`));
			});
			req.body[bodyKey] = iterator;

			next();
		};
	}

preventMaliciousBody(bodyValidationKeys: BodyValidationKeys) {
		let { mandatory = [], allowed = [] } = bodyValidationKeys;

		return function (req: Request, res: Response, next: NextFunction) {
			allowed = allowed.concat(mandatory);
			if (
				mandatory.every(value => req.body[value]) &&
				Object.keys(req.body).every(value => allowed.includes(value))
			)
				next();
			else next(createError(400, 'invalid body'));
		};
	}

createOne = catchAsync(
		async (
			req: IRemoveFieldsRequest,
			res: Response,
			next: NextFunction
		): Promise<void> => {
			const document = await this.model.create(req.body);
			if (req.removeFields) {
				req.removeFields.forEach(field => {
					document[field] = undefined;
				});
			}

			req[this.modelName] = document;

			next();
		}
	);


sendResponse = (operation: CRUD) => {
		return (req: IRequest, res: Response, next: NextFunction) => {
			switch (operation) {
				case 'create':
					res.status(201).json({
						status: 'success',
						data: req[this.modelName]
					});
					break;
			}
		};
	};


debugLog(req: IRequest, res: Response, next: NextFunction) {
		console.log(
			`${Date.now()} - ${req.url} - ParamKeys: ${Object.keys(
				req.params
			)} - BodyKeys: ${Object.keys(req.body)}`
		);
		next();
	}


// in the CollectionController
addLibraryToBody = this.moveReqKeyToBody('parent', 'library', 'id');



validateBody = {
		create: catchAsync(
			async (req: IRequest, res: Response, next: NextFunction) => {
				if (!req.body.type) req.body.type = collectionTypes.collection;
				this.preventMaliciousBody(this.bodyKeys.create)(req, res, next);

				if (
					req.body.type === collectionTypes.searchingCollection &&
					!req.body.searchQuery
				)
					next(createError(400, 'invalid body'));
				else next();
			}
		)
}

and this is my app.js:

app
	.use('/api', apiRouter)
	.use(
		'*',
		function (req: Request, res: Response, next: NextFunction) {
			// todo fix this weird bug
			if (res.headersSent) {
				console.log(req.url);
				console.log(req.body);
				console.log(req.params);
			} else next();
		},
		Controller.unavailable
	)
	.use(errorHandler);

this is postman's output on that route:

{
"status": "success"
}

this is the server's cli output:
I have morgan middleware running(first line)

POST /api/libraries/6447a4c4dc088d6d43204668/collections 201 6.358 ms - 20
1683371139354 - / - ParamKeys: id - BodyKeys: name,parent,type
/
{
name: 'norma coll',
parent: '6447a4c4dc088d6d43204668',
type: 'Collection'
}
{ '0': '/api/libraries/6447a4c4dc088d6d43204668/collections' }

I tried logging everything, and searched the web but didn't understand what's wrong.


EDIT: I added 2 console.logs to the createOne method:

createOne = catchAsync(
		async (
			req: IRemoveFieldsRequest,
			res: Response,
			next: NextFunction
		): Promise<void> => {
			console.log('started creating...');
			const document = await this.model.create(req.body);
			if (req.removeFields) {
				req.removeFields.forEach(field => {
					document[field] = undefined;
				});
			}

			req[this.modelName] = document;
			console.log('created');

			next();
		}
	);

and it was printed before and after the morgan's log:

started creating...
POST /api/libraries/6447a4c4dc088d6d43204668/collections 201 23.049 ms - 20
created
1683387954094 - / - ParamKeys: id - BodyKeys: name,parent,type
/
{
name: 'nor coll',
parent: '6447a4c4dc088d6d43204668',
type: 'Collection'
}
{ '0': '/api/libraries/6447a4c4dc088d6d43204668/collections' }

So I guess there's a problem here? maybe it doesn't get executed completely and therefore calls the next middlewares?

答案1

得分: 0

It looks like the main control problem is in validateBody.create(). You are executing:

这看起来主要的控制问题出现在validateBody.create()。你正在执行:

this.preventMaliciousBody(this.bodyKeys.create)(req, res, next);

但是,假设它是同步的,因为你继续执行它之后的代码。然而,实际情况是,它只有在其内部实现调用next()时才完成。你可以通过发送自己的回调函数而不是实际的next来使用嵌套中间件,这样你可以通过观察回调何时被调用来判断它实际上何时完成。

validateBody = {
    create: catchAsync(
        async (req: IRequest, res: Response, next: NextFunction) => {
            if (!req.body.type) req.body.type = collectionTypes.collection;
            this.preventMaliciousBody(this.bodyKeys.create)(req, res, (err) => {
                if (err) {
                    next(err);
                    return;
                }
                if (
                    req.body.type === collectionTypes.searchingCollection &&
                    !req.body.searchQuery
                ) {
                    next(createError(400, 'invalid body'));
                } else {
                    next();
                }

            }));

        }
    )
}

There are also several coding errors in moveReqKeyToBody().

moveReqKeyToBody()中还有一些编码错误。

Inside a .forEach(), you can call next(createError(400, ...)); multiple times, and then after the .forEach() loop, you then call next(). You need to code it so that next(...) is only ever called once.

.forEach()中,你可以多次调用next(createError(400, ...));,然后在.forEach()循环之后再调用next()。你需要编写代码,以确保next(...)只被调用一次。

// in the parent Controller class
moveReqKeyToBody(bodyKey: string, ...nestedReqKey: string[]) {
    return function (req: IRequest, res: Response, next: NextFunction) {
        let iterator: any = req;
        for (let key: any of nestedReqKey) {
            if (iterator[key]) {
                iterator = iterator[key];
            } else {
                next(createError(400, `missing item from request: ${nestedReqKey}`));
                return;
            }
        }
        req.body[bodyKey] = iterator;

        next();
    };
}

I also notice that sendResponse() doesn't do anything if the switch operation is not create. You should at least have a default arm to that switch that sends a response or triggers an error. If you're always expecting the argument to be create, then you wouldn't even need the switch statement. So, it should be one or the other.

我还注意到sendResponse()在开关操作不是create时不执行任何操作。你至少应该在该开关语句中添加一个default分支来发送响应或触发错误。如果你总是期望参数是create,那么你甚至不需要switch语句。所以,应该选择其中一种方式。

英文:

It looks like the main control problem is in validateBody.create(). You are executing:

this.preventMaliciousBody(this.bodyKeys.create)(req, res, next);

But, assuming that it's synchronous as you continue executing right after it. Instead, it isn't done until it calls next() internal to its implementation. You can use nested middleware like this by sending your own callback for next instead of the actual next. That way, you can see when it's actually done by when your callback gets called.

validateBody = {
create: catchAsync(
async (req: IRequest, res: Response, next: NextFunction) => {
if (!req.body.type) req.body.type = collectionTypes.collection;
this.preventMaliciousBody(this.bodyKeys.create)(req, res, (err) => {
if (err) {
next(err);
return;
}
if (
req.body.type === collectionTypes.searchingCollection &&
!req.body.searchQuery
) {
next(createError(400, 'invalid body'));
} else {
next();
}
}));
}
)
}

There are also several coding errors in moveReqKeyToBody().

Inside a .forEach(), you can call next(createError(400, ...)); multiple times and then after the .forEach() loop, you then call next(). You need to code it so that next(...) is only ever called once.

Though I can't really tell what this function is supposed to be doing or if this is entirely responsible for the problem you are asking about, you can start by fixing the above two issues like this:

// in the parent Controller class
moveReqKeyToBody(bodyKey: string, ...nestedReqKey: string[]) {
return function (req: IRequest, res: Response, next: NextFunction) {
let iterator: any = req;
for (let key: any of nestedReqKey) {
if (iterator[key]) {
iterator = iterator[key];
} else {
next(createError(400, `missing item from request: ${nestedReqKey}`));
return;
}
}
req.body[bodyKey] = iterator;
next();
};
}

I also notice that sendResponse() doesn't do anything if the switch operation is not create. You should at least have a default arm to that switch that sends a response or triggers an error. If you're always expecting the argument to be create, then you wouldn't even need the switch statement. So, it should be one or the other.

huangapple
  • 本文由 发表于 2023年5月7日 00:47:59
  • 转载请务必保留本文链接:https://go.coder-hub.com/76190059.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定