felanios / murlock

MurLock: A distributed locking solution for NestJS, providing a decorator for critical sections with Redis-based synchronization. Ideal for microservices and scalable applications.
https://www.npmjs.com/package/murlock
MIT License
62 stars 1 forks source link

Updated to latest Murlock version 3.2.0, brings a new bug (AsyncStorageManagerException). #40

Closed OsoianMarcel closed 2 months ago

OsoianMarcel commented 6 months ago

Calling the Murlock service - await this.murLockService.lock(lockName, 5e3);

Throws this error:

{
  "type": "AsyncStorageManagerException",
  "message": "No active store found",
  "stack":
      AsyncStorageManagerException: No active store found
          at AsyncStorageManager.get (/home/node/app/node_modules/murlock/lib/als/als-manager.ts:24:13)
          at MurLockService.getClientId (/home/node/app/node_modules/murlock/lib/murlock.service.ts:55:45)
          at MurLockService.<anonymous> (/home/node/app/node_modules/murlock/lib/murlock.service.ts:121:27)
          at Generator.next (<anonymous>)
          at /home/node/app/node_modules/murlock/dist/murlock.service.js:20:71
          at new Promise (<anonymous>)
          at __awaiter (/home/node/app/node_modules/murlock/dist/murlock.service.js:16:12)
          at MurLockService.unlock (/home/node/app/node_modules/murlock/dist/murlock.service.js:118:16)
          ...
          at processTicksAndRejections (node:internal/process/task_queues:95:5)
  "name": "AsyncStorageManagerException"
}

The prev. version 3.0.1 worked just fine.

felanios commented 6 months ago

Hi @OsoianMarcel, which node version are you using? Also, did you encounter this problem while using decorator?

powerofsoul commented 6 months ago

I do experience the same issue. I am using the decorator and I have node v18.19.0. Version 3.0.1 works perfectly but version 3.2.0 just throws almost the same error as op. I think the difference is that he is using the service while I am using the decorator

err: {
      "type": "MurLockException",
      "message": "Failed to acquire lock for key RafflePeriodicalCreationService:generateRafflesIfNeeded: No active store found",
      "stack":
          MurLockException: Failed to acquire lock for key RafflePeriodicalCreationService:generateRafflesIfNeeded: No active store found
              at /Users/florinmunteanu/work/tron/apps/core/node_modules/murlock/lib/decorators/murlock.decorator.ts:86:11
              at Generator.throw (<anonymous>)
              at rejected (/Users/florinmunteanu/work/tron/apps/core/node_modules/murlock/dist/decorators/murlock.decorator.js:6:65)
      "name": "MurLockException"
    }
OsoianMarcel commented 6 months ago

Hi @OsoianMarcel, which node version are you using? Also, did you encounter this problem while using decorator?

Node version: v20.11.0 I'm using the service, not decorator.

Scoup commented 5 months ago

I'm experiencing the same issue with 3.1.0 or 3.2.0. I'm using version 3.1.0-beta.1, which works just fine.

However, the issue is related to this. The only way to have a store is by using the interceptor.

If the application uses the service directly or is not using the AsyncStorageInterceptor interceptor, the store will not exist.

This approach was introduced to address this issue. However, it's a very specific scenario where the same application needs to lock the same context. Therefore, it probably should be applied only to those who have this requirement (manually), such as by adding a decorator to the endpoint or something similar.

Scoup commented 5 months ago

I think maybe a solution could be to use the nestjs-cls again.

Since this issue is related to the context, this could be used on a decorator's "group".

Something like this:

export function MurLockWrapperExample(ms: number, name: string) {
  return applyDecorators(
    UseCls(),
    MurLock(ms, name)
  );
}
felanios commented 4 months ago

Hello @Scoup @powerofsoul @OsoianMarcel , I am very sorry that I could not solve the problem due to my health problems. I have released and tested a beta version for the solution, if you test it too, I can upgrade to the stable version.

https://www.npmjs.com/package/murlock/v/3.2.1-beta.1

Scoup commented 4 months ago

@felanios I hope you're better. I have a lot of e2e tests on my application and mostly failed with this version, the same issue:

Unknown Error: Failed to acquire lock for key CacheService:updateUserReactions:b89e0d19-632b-48d3-97b3-3d873fbc964c:3fad9016-7790-4d2d-bace-068b86f0bf5f: No active store found

This is mostly a web socket project and doesn't use HTTP interceptors. I only use the decorator, like this:

  @MurLock(500, 'reaction.widgetId', 'reaction.itemId', 'reaction.userId')
  async updateReaction(reaction: Reaction) {
    const key = this.getReactionKey(reaction);
    await this.redis.set(key, JSON.stringify(reaction), 'EX', this.ttl);
  }
felanios commented 4 months ago

@Scoup Thank you I'm better now. Yes you are right, Murlock is not compatible with non-HTTP clients. Normally with nestjs-cls package it support non-HTTP clients, but nestjs-cls also come up with a different dependency problem.

Scoup commented 4 months ago

@Scoup Thank you I'm better now. Yes you are right, Murlock is not compatible with non-HTTP clients. Normally with nestjs-cls package it support non-HTTP clients, but nestjs-cls also come up with a different dependency problem.

What are the issues with nestjs-cls? The idea is to create a wrapper around some code and keep unique identifiers. Maybe we only need to think about where these wrappers will be, like in the interceptor and/or in the decorators.

felanios commented 4 months ago

@Scoup https://github.com/felanios/murlock/issues/18 This is the issue about nestjs-cls. With 3.0.0 also I switched to mount interceptor for non-http requests support and nestjs-cls perfectly support for this kind of clients.

felanios commented 3 months ago

Hey guys, sorry for late update. https://www.npmjs.com/package/murlock/v/3.3.0-beta.1 I just released new beta version for non-http clients support. I'll just keep it as a beta version for now.

Scoup commented 3 months ago

Conflict with the AsyncLocalStorage (i think) #41

Great news! Thanks for the update!

felanios commented 3 months ago

Hey @Scoup, I released the stable version also(https://www.npmjs.com/package/murlock/v/3.3.0). There were no problems when I tested it, but I still can't be sure what people will encounter. If you encounter any problems, I would appreciate it if you update this issue.

felanios commented 3 months ago

Hey @Scoup, have you encountered any issues with the latest version? If not, I will close the issue.

Scoup commented 3 months ago

Hey @Scoup, have you encountered any issues with the latest version? If not, I will close the issue.

@felanios, no issue is working fine, and all tests passed! Thanks for the update.