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

Conflict with the AsyncLocalStorage (i think) #41

Closed roderik closed 2 months ago

roderik commented 5 months ago

It took me a whole weekend of trial and error but i narrowed it down to Murlock, just not sure why.

Setting the stage:

Symptoms:

On a validation pipe failure, e.g. minimum length of a string, the entire app exits. Now this was because of NestJS Sentry, it exits when there is an unhandled exception

I verified this with the following, and indeed, the ValidationPipe BadUserInputException was unhandled

process.on('uncaughtException', (error: Error) => {
  console.error(`Caught exception: ${error}\n` + `Exception origin: ${error.stack}`);
});

The graphql examples from nest did not have that, even with NestJS Sentry installed the same way. Removing it from my app did nothing (well no exit, but still this unhandled exception)

Commenting out bit by bit, i ended up at the Murlock module. Which i use for just locking one update function so it did not pop up in my mind that it could even be related.

Removing it makes the error stay nicely in the response of the GQL query, and the process.on above no longer fires.

Reason:

beats me, my best guess is the AsyncLocalStorage bit does something that ejects the error out of the normal flow of things.

More context:

import { ConfigService } from '@nestjs/config';
import { MurLockModule } from 'murlock';
import type { RedisClientOptions } from 'redis';
import type { QueueConfigType } from '../config/queue.config';

export const MurLockModuleInit = MurLockModule.forRootAsync({
  inject: [ConfigService],
  useFactory: (configService: ConfigService) => {
    const queueConfig = configService.getOrThrow<QueueConfigType>('queue');
    return {
      redisOptions: {
        url: `redis://${queueConfig.username}:${queueConfig.password}@${queueConfig.host}:${queueConfig.port}`,
      } as RedisClientOptions,
      wait: 1000,
      maxAttempts: 3,
      logLevel: 'log',
    };
  },
});
felanios commented 5 months ago

Hi @roderik, a few more things came about AsyncLocalStorage, I will deal with this problem this week, thank you for the issue, I will inform you as soon as possible

flodaniel commented 5 months ago

We can confirm this issue as reported above.

brandart commented 5 months ago

@felanios any update on this?

felanios commented 4 months ago

Hello @roderik @brandart @flodaniel, 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

brandart commented 4 months ago

I tested it and now this issue seems to be resolved 👍

but we still cannot use this package due to this issue: https://github.com/felanios/murlock/issues/40

felanios commented 3 months ago

@roderik @brandart @flodaniel 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.