RequestContext middleware breaks entire Koa application

[oliversalzburg]

Describe the bug
When registering a middleware to use the RequestContext, it will cause previously existing routes to be answered with a 404 reply.

To Reproduce
Register the middleware per the documentation:

application.use((context, next) => {
	RequestContext.create(this.orm.em, next);
});

Expected behavior
You can later retrieve the forked entity manager through the context and your application generally still works as expected.

Additional context
My personal understanding of the fault is: RequestContext expects next to be synchronous and will call it as such and not await the result. This allows the request to finalize without ever having been handled. The handlers will still run, but they have no effect on the result, which was already dispatched.

It appears as if the current approach is modeled specifically around express middlewares, which behave differently from Koa middlewares.

A current workaround I found for myself was to just register the EntityManager on the context (something I'd like to avoid obviously) myself:

application.use(async (context,next) => {
	context.em = this.orm.em.fork();
	await next();
});

Versions

Dependency	Version
node	14
typescript	3
mikro-orm	3.6.15
your-driver	?

[oliversalzburg]

During further research, I've also managed to find a solution with @emartech/cls-adapter, which wraps cls-hooked and provides a similar facility.

I also noticed they have two distinct middleware implementations.

[B4nan]

Weird, can you verify (by adjusting the ORM code in node_modules) that it is actually caused by this?

If so, we can add RequestContext.createAsync() that would await the next handler or something similar.

---

Btw I would not suggest to use cls-hooked (at least not for production). When I was testing it some time ago (might be more than a year, but I don't see any commits in last year either), it was causing memory leaks (just like plain async_hooks - so it might be also connected to node version you are using, up to date versions like 12+ might be fine).

[oliversalzburg]

I'm not sure if this is what you had in mind, but I was able to change the code so that the issue no longer happens. This is my approach:

  static create(em) {
    return async function(ctx, next) {
      const context = new RequestContext(em.fork(true, true));
      const d = domain_1.default.create();
      d.__mikro_orm_context = context;
      await new Promise(function(resolve, reject) {
        d.run(() => next().then(resolve).catch(reject));
      });
    }
  }

The usage was adjusted as: application.use(RequestContext.create(this.orm.em));. I'm not sure I understand enough of how the domain module works to judge if that still achieves the same goals as the express implementation regarding isolation. I mostly followed the example set by the cls-adapter module.

---

Thanks for the note regarding cls-hooked. That probably saved me some headaches in the long run. Preferably, I'd stick with MikroORM approaches for this specific area, but I like the idea. In the past we'd often blindly attach properties to the request and response objects. Something I'd like to avoid on this iteration.

[B4nan]

Thanks, will try to create example repository for Koa too when debugging this to ensure it really works.

[B4nan]

4.0.0-rc.0 is out with RequestContext.createAsync().

https://github.com/mikro-orm/koa-ts-example-app

[oliversalzburg]

Works great. Migration to v4 was also smooth so far.