json-rpc-engine
json-rpc-engine copied to clipboard
Allow middleware to be async, deprecate createAsyncMiddleware
This PR lets any middleware function be async, and deprecates createAsyncMiddleware. The readme has been updated.
Previously, we did not handle rejections of plain, async middleware functions. createAsyncMiddleware was created in part to solve this problem. This PR introduces a simple refactor of #runMiddleware to handle rejections by async middleware functions.
It accomplishes this using a deferred promise, which was one of the techniques used to implement createAsyncMiddleware. Meanwhile, createAsyncMiddleware has been updated in order to retain backwards compatibility.
This change should be fully backwards-compatible.
I'd recommend leaving createAsyncMiddleware around in this PR and marking it as deprecated instead, to make migrating away from createAsyncMiddleware easier.
i think bumping major here is sufficient, others will still pull in the older version with the middleware util
hmm, i actually dont like this.
the callback api and async api are different and shouldnt be mixed
better to add a engine.addAsyncMiddleware or just mandate that all middleware is async
create async middleware currently allows you to await next() and does not have an end fn. the end is implied when the async middleware completes without calling next
the callback api and async api are different and shouldnt be mixed
@kumavis this is working up to resolving that problem. In #83 (dependency: #81 (this) -> #82 -> #83) I remove next callbacks completely in favor of a (req, res, end) signature where middleware can either be sync or async.
The main reason for removing next is that the async middleware pattern of await next(); /* do stuff */; return; invariably violates the node/callback-return ESLint rule. I agree with the authors of that rule that not returning the callback is an anti-pattern.
I decided to keep end in #83 because it seemed more explicit to call a function to end the processing of a request. That was purely an editorial decision, however, and we could instead decide to end the request whenever response.result or response.error are set, or when an error is thrown in a middleware.
This library has now been migrated into the core monorepo. This PR has been locked and this repo will be archived shortly. Going forward, releases of this library will only include changes made in the core repo.
- Please push this branch to core and open a new PR there.
- Optionally, add a link pointing to the discussion in this PR to provide context.