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

Unintuitive error when setting explicit isolation level with dialect set to postgres #55

Closed
aderant-smcd opened this issue Sep 30, 2024 · 2 comments

Comments

@aderant-smcd
Copy link

aderant-smcd commented Sep 30, 2024

If I call 'trx = await knex.transaction()', and then use it on a subsequent query via 'queryBuilder.transacting()' everything works fine.

If I call the default dialect, and calling 'knex.transaction({ isolationLevel: "serializable" })' I get a clear error - 'SET TRANSACTION ISOLATION LEVEL serializable; - Mock handler not found' - easy, I add the mock handler, everything good.

If I use the pg dialect with no explicit isolation level it also works.

So far so good, but...

If I specify an isolation level when using the pg dialect I get the following error instead: 'Transaction query already complete, run with DEBUG=knex:tx for more info'.

I spent quite a while trying to work out what was going on before switching the dialect to the default and seeing the mock handler not found error. Switching back to the 'pg' dialect and also adding a tracker.on.any call fixed it (see tests below).

Mostly just posting this issue so there's something for people like me googling this problem in future to find, but I guess the sensible fix would be either automatically mocking out transaction isolation levels so that there isn't this disconnect, or somehow adding more context to the errors from dialects that don't give useful information for things like this?

import knex, { Knex } from "knex";
import { createTracker, MockClient, Tracker } from "knex-mock-client";

describe("transaction", () => {
    let db: Knex;
    let tracker: Tracker;

    beforeAll(() => {
        db = knex({
            client: MockClient
            dialect: "pg" //<- comment out to get better error
        });
        tracker = createTracker(db);
    });

    afterEach(() => {
        tracker.reset();
    });

    it("should support transactions", async () => {
        tracker.on.insert("table_name").responseOnce(1);
        tracker.on.delete("table_name").responseOnce(1);
        tracker.on.select("foo").responseOnce([]);

        const trx = await db.transaction();

        await db("table_name").transacting(trx).insert({ name: "Steve" }).transacting(trx);
        await db("table_name").transacting(trx).delete().where({ name: "Steve" }).transacting(trx);

        await trx.commit();
    });

    it("should support transactions with an explicit isolation level", async () => {
        tracker.on.insert("table_name").responseOnce(1);
        tracker.on.delete("table_name").responseOnce(1);
        tracker.on.select("foo").responseOnce([]);

        //below makes this work when dialect is default.
        //tracker.on.any("SET TRANSACTION ISOLATION LEVEL serializable").responseOnce(undefined);
        //when dialect is pg:
        //tracker.on.any("BEGIN TRANSACTION ISOLATION LEVEL repeatable read").responseOnce(undefined);

        const trx = await db.transaction({ isolationLevel: "serializable" });

        await db("table_name").transacting(trx).insert({ name: "Steve" }).transacting(trx);
        await db("table_name").transacting(trx).delete().where({ name: "Steve" }).transacting(trx);

        await trx.commit();
    });
});
@aderant-smcd aderant-smcd changed the title Unintuitive error when setting explicit isolation level with dialect set to 'postgres' Unintuitive error when setting explicit isolation level with dialect set to postgres Sep 30, 2024
@aderant-smcd
Copy link
Author

Additionally worth mentioning it seems that the transaction query/commit/rollback tracking only works when there is no explicit isolation level provided - mocking the call makes the code run without erroring, but the transaction is not tracked.

@felixmosh
Copy link
Owner

felixmosh commented Oct 7, 2024

Fix released in V3.0.2
You should not provide a mock handler for this command :]
Thank you for your tests (were added to our test suite)

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