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

interactiveTransactions rollback but don't error out if timeout is reached #9533

Closed
awinograd opened this issue Sep 30, 2021 · 6 comments · Fixed by #10797
Closed

interactiveTransactions rollback but don't error out if timeout is reached #9533

awinograd opened this issue Sep 30, 2021 · 6 comments · Fixed by #10797
Assignees
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. tech/engines Issue for tech Engines. tech/typescript Issue for tech TypeScript. topic: interactiveTransactions
Milestone

Comments

@awinograd
Copy link

Bug description

When using interactive transactions, the transaction returns successfully even if the timeout was reached and internally the transaction was rolled back.

More context here #8664 (comment)

How to reproduce

  1. Clone https://github.com/awinograd/prisma-include-clause-bug/tree/interactive-transaction-timeout (note it is a specific branch, not main branch)
  2. run yarn dev:database
  3. run prisma migrate dev
  4. run yarn dev

Expected behavior

Transaction that surpasses timeout should throw an error rather than returning the return value as if nothing has happened.

Prisma information

all included in repro example above

Environment & setup

  • OS: MacOS
  • Database: PostgresQL
  • Node.js version: 16.9.1

Prisma Version

prisma                  : 3.1.1
@prisma/client          : 3.1.1
Current platform        : darwin
Query Engine (Node-API) : libquery-engine c22652b7e418506fab23052d569b85d3aec4883f (at node_modules/@prisma/engines/libquery_engine-darwin.dylib.node)
Migration Engine        : migration-engine-cli c22652b7e418506fab23052d569b85d3aec4883f (at node_modules/@prisma/engines/migration-engine-darwin)
Introspection Engine    : introspection-core c22652b7e418506fab23052d569b85d3aec4883f (at node_modules/@prisma/engines/introspection-engine-darwin)
Format Binary           : prisma-fmt c22652b7e418506fab23052d569b85d3aec4883f (at node_modules/@prisma/engines/prisma-fmt-darwin)
Default Engines Hash    : c22652b7e418506fab23052d569b85d3aec4883f
Studio                  : 0.423.0
Preview Features        : interactiveTransactions
@awinograd awinograd added the kind/bug A reported bug. label Sep 30, 2021
@pantharshit00
Copy link
Contributor

I can reproduce this. Thanks for the detailed report.

@Gerrit-K
Copy link

Gerrit-K commented Oct 26, 2021

Can this affect regular (non-interactive) transactions as well? We have enabled interactiveTransactions in our client for one specific call (which is very unlikely to time out, though), but we're currently investigating an issue very similar to this one, where a prisma.$transaction([...]) call does not throw or log any errors, but the supposedly created data is not present in the database afterwards. Replacing prisma.$transaction([...]) with Promise.all([...]) solved this (and fortunately for us, we don't really need transactions in this context).

Edit: upon second glance, I thought my question might be a bit vague, so let me rephrase it:
With respect to this issue, is there a difference between prisma.$transaction([...]) and prisma.$transaction(async (prisma) => {...}) or are both variants affected by the issue if interactiveTransactions is enabled?

@avallete
Copy link
Contributor

Can this affect regular (non-interactive) transactions as well? We have enabled interactiveTransactions in our client for one specific call (which is very unlikely to time out, though), but we're currently investigating an issue very similar to this one, where a prisma.$transaction([...]) call does not throw or log any errors, but the supposedly created data is not present in the database afterwards. Replacing prisma.$transaction([...]) with Promise.all([...]) solved this (and fortunately for us, we don't really need transactions in this context).

Edit: upon second glance, I thought my question might be a bit vague, so let me rephrase it: With respect to this issue, is there a difference between prisma.$transaction([...]) and prisma.$transaction(async (prisma) => {...}) or are both variants affected by the issue if interactiveTransactions is enabled?

I think this issue is related to another one I opened here: #9584

If the root cause is indeed the same (issue in transaction timeout handling), it should affect regular transactions as well, since in my issue I made an MRE reproducing the error with "regular transactions".

@matthewmueller matthewmueller added tech/engines Issue for tech Engines. tech/typescript Issue for tech TypeScript. labels Nov 29, 2021
@matthewmueller matthewmueller added this to the 3.7.0 milestone Nov 30, 2021
@matthewmueller matthewmueller modified the milestones: 3.7.0, 3.8.0 Dec 21, 2021
@garrensmith
Copy link
Contributor

From what I can see on this. The issue is here

await this.engine?.commitTransaction(arg.id, '{}')

The query-engine will never throw an actual error but will return an error object:

{
  "is_panic":false,
  "message":"Transaction API error: Transaction already closed: Transaction is no longer valid. Last state: 'Expired'.",
   "meta":{
        "error":"Transaction already closed: Transaction is no longer valid. Last state: 'Expired'."
    },
 "error_code":"P2028"
}

We need to check the response and throw an error if we detect it. We need to do the same for rollback

@millsp millsp modified the milestones: 3.8.0, 3.9.0 Jan 11, 2022
@matthewmueller
Copy link
Contributor

matthewmueller commented Jan 11, 2022

PR ready for review here: #10797

@livthomas
Copy link

I think I've also run into this issue. When I turn on interactiveTransactions preview feature, a long-running transaction invoked the old way (prisma.$transaction([...])) suddenly starts failing with this error:

[0] prisma:info Starting a sqlite pool with 9 connections.
[0] PrismaClientKnownRequestError: 
[0] Invalid `prisma_1.prisma.contract.create()` invocation in
[0] /home/livthomas/Projects/livthomas/pawnshop-next/build/main/legacy/contract/importLegacyContractRecords.js:15:41
[0] 
[0]   12 const userPasswordsMap = await (0, createUserHashedPasswordsMap_1.createUserHashedPasswordsMap)(userNames);
[0]   13 const operations = results.map(result => {
[0]   14     const data = (0, convertLegacyRecordToContract_1.convertLegacyRecordToContract)(result, userPasswordsMap, archived);
[0] → 15     return prisma_1.prisma.contract.create(
[0]   Transaction API error: Transaction already closed: Transaction is no longer valid. Last state: 'Expired'.
[0]     at Object.request (/home/livthomas/Projects/livthomas/pawnshop-next/node_modules/@prisma/client/runtime/index.js:39054:15)
[0]     at async Promise.all (index 0)
[0]     at async PrismaClient._transactionWithCallback (/home/livthomas/Projects/livthomas/pawnshop-next/node_modules/@prisma/client/runtime/index.js:39655:18)
[0]     at async importLegacyContractRecords (/home/livthomas/Projects/livthomas/pawnshop-next/build/main/legacy/contract/importLegacyContractRecords.js:17:5)
[0]     at async reportLegacyTableImportStatus (/home/livthomas/Projects/livthomas/pawnshop-next/build/main/legacy/reportLegacyTableImportStatus.js:34:9)
[0]     at async importLegacyDatabase (/home/livthomas/Projects/livthomas/pawnshop-next/build/main/legacy/importLegacyDatabase.js:27:20)
[0]     at async electron/js2c/browser_init.js:197:563 {
[0]   code: 'P2028',
[0]   clientVersion: '3.8.0',
[0]   meta: {
[0]     error: "Transaction already closed: Transaction is no longer valid. Last state: 'Expired'."
[0]   }
[0] }

This transaction works without any problems when this preview feature is disabled and its execution usually takes around 30 seconds.

I'm using SQLite database.

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. tech/engines Issue for tech Engines. tech/typescript Issue for tech TypeScript. topic: interactiveTransactions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants