feat: Add test for nested transaction rollback behaviour in SQL databases

[LucianBuzzo]

This adds a high level test that should pass if/when https://github.com/prisma/prisma-engines/pull/4375 is merged.

This change adds support for handling rollbacks in nested transactions in SQL databases. Specifically, the inner transaction should be rolled back if the outer transaction fails.

To do this we keep track of the transaction ID and transaction depth so we can re-use an existing open transaction in the underlying engine. This change also allows the use of the $transaction method on an interactive transaction client.

[CLAassistant]

All committers have signed the CLA.

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #21678 will not alter performance

_{Comparing LucianBuzzo:lucianbuzzo/nested-rollbacks (e5f4192) with main (9810341)}

Summary

✅ 3 untouched benchmarks

[LucianBuzzo]

@Jolg42 Is it possible to verify that https://github.com/prisma/prisma-engines/pull/4375 fixes the issue and gets the test to pass?

[janpio]

~~@prisma/engines-version@5.6.0-12.integration-sql-nested-transactions-a8af640da343c10e0f7421264c2838b302d3ce6d via https://github.com/prisma/prisma/pull/21735/files should work for that.~~

Update: That surfaced that the PR in prisma-engines is not up do date with main, which makes some tests fail. We are fixing that right now and then will have a new integration version to test here.

Update 2: @prisma/engines-version@5.6.0-14.integration-sql-nested-transactions-5a0f60fe78f2029f692e21ce8732a978466b9baf might be the one, via https://github.com/prisma/prisma/pull/21737

[janpio]

Ummm, seems it is not working @LucianBuzzo. Or I messed up royally. Asking some colleagues for advice.

[LucianBuzzo]

@janpio I'll look into it on my end - any idea how to run just the tests/functional/interactive-transactions/tests.ts file locally?

[LucianBuzzo]

@janpio I'm replicating the issue on my end - I think I might need to update the client code to maintain the same transaction ID in nested transactions 🤔 . I'm going to dig through and see what I can come up wiht.

[janpio]

Can be updated to use version from https://github.com/prisma/prisma/pull/22161 now

[LucianBuzzo]

@janpio The tests are passing locally. I'm now working on an edge case where you use a batch transaction inside a client extensions 😬

[Jolg42]

@LucianBuzzo to get the CI running, you would need to solve the git conflict. GitHub won't run any Action if there is a conflict with the base branch unfortunately 😢

[LucianBuzzo]

@Jolg42 Cool, I'll get that sorted 👍

[LucianBuzzo]

@Jolg42 I'm getting a lot of tests failing with an error about relationJoins, e.g.

issues.16535-select-enum (provider=mysql) › allows to select enum field

    GetDmmfError: Prisma schema validation - (get-dmmf wasm)
    Error code: P1012
    error: The preview feature "relationJoins" is not known. Expected one of: deno, driverAdapters, fullTextIndex, fullTextSearch, metrics, multiSchema, postgresqlExtensions, tracing, views
      -->  schema.prisma:6
       | 
     5 | engineType = "library"
     6 | previewFeatures = ["relationJoins"]
       | 
       

Any idea why this is happening?

EDIT: I'm going to guess that the engine version I'm using doesn't have this feature - I'll work on getting the engine PR updated

[Jolg42]

@LucianBuzzo

EDIT: I'm going to guess that the engine version I'm using doesn't have this feature - I'll work on getting the engine PR updated

Exactly, the main branch evolves rapidly, so getting your fork in sync would work I think.

[LucianBuzzo]

The new interactive rollback tests are now passing and behaving as expected, however there are now some failing tests elsewhere that I'll need to address.

[LucianBuzzo]

From what I can tell the transaction depth isn't getting decremented when using the js_pg adapter. I've added a test to sanity check this:

 pnpm run test:functional --adapter js_pg --shard 6/6 --client-runtime wasm --preview-features driverAdapters --runInBand -t 'multiple interactive transactions'

I think this is an issue at the engine level, so I will update my PR there https://github.com/prisma/prisma-engines/pull/4375

[LucianBuzzo]

@SevInf It looks like everything is now passing, except d1 and mini-proxy, which ~~looks to be unrelated~~ I'll dig into 😅

[SevInf]

mini-proxy might actually be related: did we change binary-engine public interface in the engine PR? Both mini-proxy and real Accelerate use it underneath, so we might take a closer look to ensure they won't break once it released. Of course, both of those projects are in the private repos, so there is nothing really actionable for you right now. I'll try to carve out some time to take a look myself, but realistically, that probably happen after 5.12.0 release next week.

[LucianBuzzo]

@SevInf I've added a begin() method in the engine that is used to open the next transaction level on a transaction that has already been started. That might be the culprit. If there's nothing I can do to help, I'll sit tight until after the v5.12.0 release 👍

[LucianBuzzo]

@SevInf Now that v5.12.0 is released, are you able to assist in the mini-proxy and d1 test failures? TIA 🙏

[LucianBuzzo]

@SevInf @janpio Any updates on how to move this PR forwards?

[socket-security[bot]]

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

[janpio]

@LucianBuzzo I contacted you via LinkedIn (only method I could find) to talk about how getting a better way for us to talk to each other, to figure out the next steps (Either invite you to our Slack, or via Discord?).

I merged, which, of course, led to a few more tests failing - 417db07 (#21678) is the last commit before I did that to get to the test failures that probably exist and are real, and not just caused by new tests that the engine version here does not support yet.

[LucianBuzzo]

@janpio Thanks, I'll check out the test failures - I've responded to you message on LinkedIn, slack is probably the best bet 👍

[LucianBuzzo]

@SevInf Looks like the mini-proxy tests were failing because I was accidentally overwriting the transaction payload when making a request to the engine. This is fixed and everything appears to be working correctly. I think the Client func&legacy-notypes might be flakey - can you rerun the failed test?