express-json-validator-middleware
express-json-validator-middleware copied to clipboard
Allow Promises of schemas
This is a retread of #31, but with some actually concrete proposals and rationale.
Currently, it is only possible to provide either literal schemas, or schemas from synchronous functions. If a schema requires (e.g.) a database read, the only way to do it is to perform the operation in a prior middleware, then pass it along by mutating the request object (or to wrap this middleware in something custom). This is sub-optimal for a few reasons:
- It separates operations that should be performed together. Having to put together two separate middlewares to perform your validation cycle makes it highly likely that you'll eventually forget one and break something.
- Modifying the request object is problematic in TypeScript. There's no good place to attach a random value of arbitary type, and you lose the type information in the following middlewares.
- Multiple middlewares modifying the request object may stomp each other's keys, producing an unknown state by the time it reaches the schema generator.
My proposed solution to this is fairly straightforward: allow passing Promises of a schema, or functions that return Promises of a schema. In TypeScript terms, changing this (borrowed from #72 for easier readability):
type AllowedSchema =
| JSONSchema4
| JSONSchema6
| JSONSchema7;
export type ValidateFunction =
| ((req: Request) => AllowedSchema)
| AllowedSchema;
to this:
type AllowedSchema =
| JSONSchema4
| JSONSchema6
| JSONSchema7;
export type ValidateFunction =
| ((req: Request) => AllowedSchema | Promise<AllowedSchema>)
| Promise<AllowedSchema>
| AllowedSchema;
Then, the request handler can await the Promise, and perform the normal operation.
@NGTOne Thanks for raising this issue - the rationale you've provided for this feature is sound.
I agree that it's not a good practice to mutate the request object. The coupling of middleware is not ideal either. I would love to deprecate the currently documented approach to loading dynamic schemas.
My one concern with this proposal is the need to be especially careful with Express and Promises, given that Express does not have out-of-the-box support for handling rejected promises.
Then, the request handler can
awaitthe Promise, and perform the normal operation.
What are you referring to as "the request handler" in this context?
My one concern with this proposal is the need to be especially careful with Express and Promises, given that Express does not have out-of-the-box support for handling rejected promises.
Ultimately, I'd argue that that one falls into the "not our problem" bucket. As long as all errors are caught by the middleware and tossed into next(), either with a try/catch or #Promise.catch, then the consumer can decide what to do with them later on down the line. The same holds true for synchronous functions, to my mind - if you throw something there, interrupting the request cycle is rather decidedly not the right approach.
What are you referring to as "the request handler" in this context?
The actual returned (req, res, next) function. It can either await the Promise and try/catch any thrown errors, or just slap a .catch on it. Either way, it can then feed the resulting error into next() and let the consumer deal with it.
My one concern with this proposal is the need to be especially careful with Express and Promises, given that Express does not have out-of-the-box support for handling rejected promises.
Ultimately, I'd argue that that one falls into the "not our problem" bucket. As long as all errors are caught by the middleware and tossed into
next(), either with a try/catch or#Promise.catch, then the consumer can decide what to do with them later on down the line. The same holds true for synchronous functions, to my mind - if you throw something there, interrupting the request cycle is rather decidedly not the right approach.
I agree - I wasn't sure if you were proposing that the express-json-validator-middleware have error handling for Promises which are passed to it.
What are you referring to as "the request handler" in this context?
The actual returned
(req, res, next)function. It can eitherawaitthe Promise and try/catch any thrown errors, or just slap a.catchon it. Either way, it can then feed the resulting error intonext()and let the consumer deal with it.
This sounds good to me :+1:
If this is a feature that you're happy to work on, I would love to see it land in a release in the near future.
At the moment I'm close to getting v2.2.0 released (#39 is the only blocking issue that I need to spend more time investigating). After that release I want to do a v3.0.0 release which will upgrade Ajv to v7. I'm thinking that this feature could go into a subsequent release.
Here are the release milestones: https://github.com/simonplend/express-json-validator-middleware/milestones