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

$transaction doesn't roll back in NestJS when we pass method from external service #5730

Closed
revivalme opened this issue Feb 18, 2021 · 6 comments
Assignees
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. topic: prisma-client topic: transaction
Milestone

Comments

@revivalme
Copy link

revivalme commented Feb 18, 2021

Bug description

$transaction works incorrectly in NestJS when we pass method from external service, but works well if it uses method from current service.
It doesn't roll back as expected.

Example:

// UserService
@Injectable()
export class UserService {
  constructor(private readonly prismaService: PrismaService) {}

  addMoney(userId: number, value: number) {
    return this.prismaService.user.update({
      where: { id: userId },
      data: {
        money: {
          increment: value,
        },
      },
    });
  }
}

// InventoryService
@Injectable()
export class InventoryService {
  constructor(
    private readonly prismaService: PrismaService,
    private readonly userService: UserService,
  ) {}

  async sell(userId: number, itemId: number) {
    const write1 = this.prismaService.userItem.delete({
      where: { ... },
    });

    // external service method
    const write2 = this.userService.addMoney(...);

    await this.prismaService.$transaction([write1, write2]);
  }

  addMoney(userId: number, value: number) {
    return this.prismaService.user.update({
      where: { id: userId },
      data: {
        money: {
          increment: value,
        },
      },
    });
  }
}

If write2 will fail - write1 doesn't rollback.
But... rollback will work as expected if write2 refers to the current service method. (this.addMoney)

How to reproduce

https://github.com/revivalme/nestjs-prisma-transaction-test
Bug reproduction section

Expected behavior

Transaction write1 should rollback as transaction write2 failed.

Prisma information

Environment & setup

  • OS: Windows 10
  • Database: SQLite, PostgreSQL
  • Node.js version: v14.15.4
  • Prisma version:
prisma               : 2.17.0
@prisma/client       : 2.17.0
Current platform     : windows
Query Engine         : query-engine 3c463ebd78b1d21d8fdacdd27899e280cf686223 (at node_modules\@prisma\engines\query-engine-windows.exe)
Migration Engine     : migration-engine-cli 3c463ebd78b1d21d8fdacdd27899e280cf686223 (at node_modules\@prisma\engines\migration-engine-windows.exe)
Introspection Engine : introspection-core 3c463ebd78b1d21d8fdacdd27899e280cf686223 (at node_modules\@prisma\engines\introspection-engine-windows.exe)
Format Binary        : prisma-fmt 3c463ebd78b1d21d8fdacdd27899e280cf686223 (at node_modules\@prisma\engines\prisma-fmt-windows.exe)
Studio               : 0.353.0

Conversation from Public Slack here

@Jolg42 Jolg42 added bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. kind/bug A reported bug. process/candidate team/client Issue for team Client. topic: prisma-client topic: transaction labels Feb 18, 2021
@timsuchanek
Copy link
Contributor

Thanks @revivalme for the report. We'll have a look at this!

@pantharshit00
Copy link
Contributor

I can confirm this. From the logs, it looks like transaction flag is not set correctly here and writes are not running in a transaction.

image

@pantharshit00 pantharshit00 added bug/2-confirmed Bug has been reproduced and confirmed. and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Mar 2, 2021
@matthewmueller matthewmueller added this to the 2.19.0 milestone Mar 3, 2021
@williamluke4
Copy link
Contributor

williamluke4 commented Mar 17, 2021

@revivalme Could you try out williamluke4:patch-1 ?

@revivalme
Copy link
Author

@revivalme Could you try out williamluke4:patch-1 ?

Yeah, for some unknown reason it works, but shouldn't. UserModule has exports array with UserService, so whenever I import UserModule into other modules - I already have access to UserService without manually add this service to providers array.

(same info here https://docs.nestjs.com/modules#shared-modules)

The especially weird part of this is that in both cases, with/without manually add service to providers array, addMoney method of UserService invokes.

Also both cases have similar errors:

[Just with module import]

Invalid `prisma.user.update()` invocation:


  An operation failed because it depends on one or more records that were required but not found. Record to update not found. +5117ms
Error: 
Invalid `prisma.user.update()` invocation:


  An operation failed because it depends on one or more records that were required but not found. Record to update not found.
    at cb (C:\Users\revivalme\Desktop\nestjs-prisma-transaction-test\node_modules\@prisma\client\runtime\index.js:78689:17)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Promise.all (index 1)
    at async InventoryService.sellItem (C:\Users\revivalme\Desktop\nestjs-prisma-transaction-test\dist\src\inventory\inventory.service.js:56:9)
    at async C:\Users\revivalme\Desktop\nestjs-prisma-transaction-test\node_modules\@nestjs\core\router\router-execution-context.js:46:28
    at async C:\Users\revivalme\Desktop\nestjs-prisma-transaction-test\node_modules\@nestjs\core\router\router-proxy.js:9:17

[Module import with manual add service to providers]

InterpretationError("Error for binding \'0\'", Some(QueryGraphBuilderError(RecordNotFound("Record to update not found.")))) +5902ms
Error: Error occurred during query execution:
InterpretationError("Error for binding \'0\'", Some(QueryGraphBuilderError(RecordNotFound("Record to update not found."))))
    at C:\Users\revivalme\Desktop\nestjs-prisma-transaction-test\node_modules\@prisma\client\runtime\index.js:27103:19
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

So, given the above, guess it's too early to close this issue

@matthewmueller matthewmueller modified the milestones: 2.19.0, 2.20.0 Mar 17, 2021
@williamluke4
Copy link
Contributor

@revivalme Both your logs above seem unrelated to this issue. They seem to be a result of you trying to update a record that does not exist in the database

An operation failed because it depends on one or more records that were required but not found.

And

Record to update not found.

@williamluke4
Copy link
Contributor

Hey @revivalme, I'm going to close this issue, if you feel like the problems you are experiencing are still related to this specific issue then feel free to reopen it explaining why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. topic: prisma-client topic: transaction
Projects
None yet
Development

No branches or pull requests

6 participants