Transactional doesn't work well with Promise.all

[preterer]

I double checked https://github.com/odavid/typeorm-transactional-cls-hooked/issues/58 and despite the fact, that I still belive the code was not written correctly, there actually is a problem with Promise.all calls if there are many nested calls to methods decorated as Transactional.

Reproduction code (needs sql.js library installed):

// test-entity.ts
// tslint:disable:max-classes-per-file

import { Service } from 'typedi';
import { Column, Entity, PrimaryGeneratedColumn, Repository } from 'typeorm';
import { Transactional } from 'typeorm-transactional-cls-hooked';
import { InjectRepository } from 'typeorm-typedi-extensions';

@Entity({ name: 'entity' })
export class TestEntity {
  @PrimaryGeneratedColumn({ name: 'id' })
  public id: number;

  @Column({ name: 'name' })
  public name: string;
}

@Service()
export class TestService {
  @InjectRepository(TestEntity)
  public repository: Repository<TestEntity>;

  @Transactional()
  public reprodA(ids: number[]): Promise<any> {
    console.log('reprodA start');
    return Promise.all(
      ids.map((id, index) => {
        const prevId = index === 0 ? ids[ids.length - 1] : ids[index - 1];
        return this.reprodB(id, prevId);
      })
    );
  }

  @Transactional()
  public workaround(ids: number[]): Promise<any> {
    console.log('workaround start');
    let error;
    return Promise.all(
      ids.map((id, index) => {
        const prevId = index === 0 ? ids[ids.length - 1] : ids[index - 1];
        return this.reprodB(id, prevId).catch(err => (error = error || err));
      })
    ).then(() => {
      if (error) {
        throw error;
      }
    });
  }

  @Transactional()
  public async reprodB(id: number, prevId: number): Promise<any> {
    if (id === 2) {
      throw new Error('Mock error');
    } else {
      await this.timeConsumingOperation();
    }
    await this.reprodC(id);
  }

  @Transactional()
  public async reprodC(id: number): Promise<any> {
    let entity = await this.repository.findOne(id);
    if (!entity) {
      entity = this.repository.create();
    }
    entity.name = 'Test';
    await this.repository.save(entity);
  }

  public timeConsumingOperation() {
    return new Promise(resolve => setTimeout(() => resolve(true), 2000));
  }
}

// promise-all.test.ts
import { Container } from 'typedi';
import * as TypeORM from 'typeorm';
import {
  initializeTransactionalContext,
  patchTypeORMRepositoryWithBaseRepository
} from 'typeorm-transactional-cls-hooked';

import { initializeCls } from '../src/server/initializeCls';
import { TestEntity, TestService } from './helpers/testEntity';

describe('Promise.all', () => {
  beforeAll(() => {
    initializeTransactionalContext();
    patchTypeORMRepositoryWithBaseRepository();
    initializeCls();
    TypeORM.useContainer(Container);
    return TypeORM.createConnection({
      cache: false,
      dropSchema: true,
      entities: [TestEntity],
      logger: 'advanced-console',
      logging: 'all',
      synchronize: true,
      type: 'sqljs'
    });
  });

  it('will commit some inner transactions on error', async () => {
    const service = Container.get(TestService);
    const testEntitiesCountBefore = await service.repository.count();
    await expect(service.reprodA([1, 2, 3, 4])).rejects.toThrow();
    await service.timeConsumingOperation();
    const testEntitiesCountAfter = await service.repository.count();
    expect(testEntitiesCountAfter).toBeGreaterThan(testEntitiesCountBefore);
  });

  it('should not commit any inner transactions on error', async () => {
    const service = Container.get(TestService);
    const testEntitiesCountBefore = await service.repository.count();
    await expect(service.workaround([5, 2, 6, 7])).rejects.toThrow();
    await service.timeConsumingOperation();
    const testEntitiesCountAfter = await service.repository.count();
    expect(testEntitiesCountAfter).toEqual(testEntitiesCountBefore);
  });
});

I'm not really sure what can you do with that in your library, as there is no way of canceling a promise.

A workaround I found is to catch the errors, let all the promises continue, and then throw the caught error when they're all finished, It is used in the workaround method of TestService.

However it is ugly and consumes time/server power for no reason, and would be great if there was a way of solving this issue in the library. Maybe it would be possible to mark the transaction as failed in CLS and just throw errors on any repository call if it's marked, I'll try to look into it.

Anyway, untill it's resolved I'll leave this issue here with a workaround.

[preterer]

Silly me. It's solvable by putting Propagation.MANDATORY on reprodC, and creating a reprodD which just calls reprodC in a started transaction, to use it in places where you can't start a transaction (like tests, or cli).

@Transactional()
public reprodD(id: number): Promise<any> {
  return this.reprodC(id);
}

@Transactional({ propagation: Propagation.MANDATORY })
public async reprodC(id: number): Promise<any> {
  let entity = await this.repository.findOne(id);
  if (!entity) {
    entity = this.repository.create();
  }
  entity.name = 'Test';
  await this.repository.save(entity);
}