Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequelize.useCLS not working? #1083

Open
1 of 3 tasks
seimic opened this issue Sep 2, 2021 · 2 comments
Open
1 of 3 tasks

Sequelize.useCLS not working? #1083

seimic opened this issue Sep 2, 2021 · 2 comments

Comments

@seimic
Copy link

seimic commented Sep 2, 2021

Issue

Configuration for CLS (Continuation-Local Storage) seems not to work or I'm doing something wrong.
CRUD operations don't join existing transaction.
Any chance to get it done right?

Versions

  • nestjs: 8.0.6
  • cls-hooked: "4.2.2"
  • sequelize: 6.6.5
  • sequelize-typescript: 2.1.0
  • typescript: 4.4.2

Issue type

  • bug report
  • feature request
  • not sure. problem maybe in front of monitor. :-)

Actual behavior

CRUD operations not joining current transaction.

Expected behavior

Opposite :)

Steps to reproduce

Configuration (Somewhere on application init, before module initialization)

import { Sequelize } from 'sequelize-typescript';
import * as cls from 'cls-hooked';

const ns = cls.createNamespace('sequelize-ns');
Sequelize.useCLS(ns);

Interceptor

import { Sequelize } from 'sequelize-typescript';
@Injectable()
export class TransactionInterceptor implements NestInterceptor {
  constructor(private readonly sequelize: Sequelize) {}

  async intercept(
    context: ExecutionContext,
    next: CallHandler,
  ): Promise<Observable<any>> {
    // Managed tx with autocommit and rollbacks on error
    return await this.sequelize.transaction(async (tx: Transaction) => {
      GqlExecutionContext.create(context).getContext().transaction = tx;
      return next.handle();
    });
  }
}

Not working version

class FooResolver {
  ...  
  @UseInterceptors(TransactionInterceptor)
  @Mutation(() => FooModel)
  async createFoo(
    @Args('dto') dto: FooInput,
  ): Promise<FooModel> {
    const foo = new FooEntity(dto);
    await foo.save(); 
    
    const foo2 = new FooEntity({ id: 1, name: null });
    await foo2.save(); // This one will fail
    
    return new FooModel(foo2);
  }  
}
Executing (b32b84c4-d552-4073-a2fa-d4a35af26d3f): START TRANSACTION;
Executing (b32b84c4-d552-4073-a2fa-d4a35af26d3f): SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
Executing (default): INSERT INTO "tbl_foo" ("id","name") VALUES ($1,$2) RETURNING "id","name"; !!! Not in transaction
Executing (default): INSERT INTO "tbl_foo" ("id","name") VALUES ($1,$2) RETURNING "id","name"; !!! Not in transaction
Executing (b32b84c4-d552-4073-a2fa-d4a35af26d3f): ROLLBACK;
[Nest] 28804  - 02.09.2021, 10:42:03   ERROR [ExceptionsHandler] Validation error
...

Working version (the uggly one, passing transaction to each statement)

class FooResolver {
 ...
  @UseInterceptors(TransactionInterceptor)
  @Mutation(() => FooModel)
  async createFoo2(
    @Context ctx: { transaction: Transaction }, <-- Tx from context
    @Args('dto') dto: FooInput,
  ): Promise<FooModel> {
    const foo = new FooEntity(dto);
    await foo.save({ transaction: ctx.transaction }); 
    
    const foo2 = new FooEntity({ id: 1, name: null });
    await foo2.save({ transaction: ctx.transaction }); // This one will fail
    
    return new FooModel(foo2);
  }  
Executing (977845cd-25ea-4350-997b-fb20b6d59030): START TRANSACTION;
Executing (977845cd-25ea-4350-997b-fb20b6d59030): SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
Executing (977845cd-25ea-4350-997b-fb20b6d59030): INSERT INTO "tbl_foo" ("id","name") VALUES ($1,$2) RETURNING "id","name";
Executing (977845cd-25ea-4350-997b-fb20b6d59030): INSERT INTO "tbl_foo" ("id","name") VALUES ($1,$2) RETURNING "id","name";
Executing (977845cd-25ea-4350-997b-fb20b6d59030): ROLLBACK;
[Nest] 77072  - 02.09.2021, 11:03:03   ERROR [ExceptionsHandler] Validation error
...

@seimic
Copy link
Author

seimic commented Sep 4, 2021

OK, it seems to be a bug. Sequelize.useCLS(ns); doesn't set the namespace in the original Sequelize class.

After some debugging... this works.
Initialization somewhere before first usage of sequelize.
See: #58 (comment)

// import * as SequelizeOrigin from 'sequelize';
import { Sequelize } from 'sequelize-typescript';
import * as cls from 'cls-hooked';
const ns = cls.createNamespace('sequelize-transaction');
// Sequelize.useCLS(ns); // Doesn't work!
// Sequelize['useCLS'](ns); // Doesn't work!
(Sequelize as any).__proto__.useCLS(ns); // This works
// (SequelizeOrigin as any).useCLS(ns); // This works too

The interceptor needs to be changed too, to return Promise<Observable> which resolves after the observable finishes.

import {
  CallHandler,
  ExecutionContext,
  Injectable,
  NestInterceptor,
} from '@nestjs/common';
import { GqlExecutionContext } from '@nestjs/graphql';
import { firstValueFrom, Observable } from 'rxjs';
import { Transaction } from 'sequelize';
import { Sequelize } from 'sequelize-typescript';

@Injectable()
export class TransactionInterceptor implements NestInterceptor {
  constructor(private readonly sequelize: Sequelize) {}

  async intercept(
    context: ExecutionContext,
    next: CallHandler,
  ): Promise<Observable<any>> {
    return this.sequelize.transaction((tx: Transaction) => {
      const ctx = GqlExecutionContext.create(context);
      ctx.getContext().transaction = tx;
      return firstValueFrom(next.handle());
    });
  }
}

Kind regards,
Michael

@ephys
Copy link
Member

ephys commented Apr 7, 2022

Is this still an issue? It should have been fixed in Sequelize 6.7.0 by sequelize/sequelize#13510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants