fork inside context - Maximum call stack size exceeded

[ml1nk]

Describe the bug
Since cc6d59b fork always calls getContext which calls the config function context if useContext is true on the used entity manager. This leads to a loop if fork is used from the global instance (which always has useContext true) inside the context function.

To Reproduce
The use of orm.em.fork inside the context function leads to "Maximum call stack size exceeded".

In my case 'Context' uses AsyncLocalStorage to store and retrieve several objects with some sort of type safety. orm.em.fork is called if the store does not already contains an EntityManager (first call).

context: () => {
      const store = Context.getStore()
      if (store === undefined || orm === null) return undefined
      return store.getOrSet(contextEmKey, EntityManager, () =>  orm!.em.fork())
 }

Current workaround:

context: () => {
      const store = Context.getStore()
      if (store === undefined || orm === null) return undefined
      return store.getOrSet(contextEmKey, EntityManager, () => {
        // @ts-expect-error
        orm!.em.useContext = false
        try {
          return orm!.em.fork()
        } finally {
          // @ts-expect-error
          orm!.em.useContext = true
        }
      })
 }

Versions

Dependency	Version
node	18.6.0
typescript	4.7.4
mikro-orm	5.2.4

[B4nan]

Why are you even doing this? The default RequestContext helper already uses ALS. It also sounds weird to me you want to call em.fork() inside that method - it should be called in some middleware instead and here you should be just getting it from somewhere.

[ml1nk]

We are creating our own instance of AsyncLocalStorage and are switching context independent of any middleware. For example a task in our system has it's own context and can create a sub context if needed which eventually needs a new fork of mikro-orm. So we cannot use RequestContext as such but the context function as shown above. As the fork function is only called if some orm.em operation is called and there isn't already an EntityManager in this context, it is more efficient than directly creating one every time a new context is created.

You are right, if i create the fork outside the context and return undefined inside the context as long as no fork is known, it would use the global instance without looping. Before the change it was possible to create a fork inside the context, now i need to use some kind of workaround.

I think it would be enough to add an property in ForkOptions which disables the getContext call inside fork.

Somewhat OT:
In addition we are using a Logger which calls upon the context to get the current debug level, which allows us to activate the query log only when something triggers the debug-level inside the context.

[B4nan]

Interesting, thanks for explaination.

I think it would be enough to add an property in ForkOptions which disables the getContext call inside fork.

Yeah we can definitely do something like that. Or have a recursion check will do the job as well.

[ml1nk]

No problem, i think we are catching all the edge cases ... always 😢

I would prefer the option (noContext?), i find a recursion checks somewhat crazy/unpredictable.

[B4nan]

Yeah I know, but using new option can be quite confusing given we have the useContext option already. Now those two would sounds quite similar but do very different things.

[B4nan]

Btw you should also be able to get around by creating first fork outside of the getContext method. On that fork, doing fork.getContext() will always return fork. Sounds like better workaround that mangling with the boolean directly.

[ml1nk]

If i create a fork outside of the method, it would call the context method which then tries to create a fork....
I could use my workaround/hack to create the first fork without triggering context.

[B4nan]

Lol right :D Lets just come up with some better name and do the option. Can be a bit longer, this is quite specific use case so better to have a descripitive name.

[ml1nk]

Ideas:
disableContextResolution
ignoreContext
forceIgnoreContext (as it override useContext of the current em)

[B4nan]

I like disableContextResolution, the other ones still sounds similar to useContext.

[ml1nk]

I think f7c1ef2 broke disableContextResolution as i get an "Maximum call stack size exceeded" again.

It still takes "this" in line 1168 but then calls getContext when schema is retrieved in line 1187 which triggers the loop.

[B4nan]

Let me know if 5964e52 helps.

[ml1nk]

Works again, thanks.