fix: fix "Cannot commit, no transaction is active" error in sql.js

[Obiwan1995]

Fixes: #9100

Description of change

When calling autoSave in a transaction, typeorm automatically closes the transaction in the export method. Therefore, when calling commitTransaction, the transaction is already committed and sqljs throws an exception "Cannot commit, no transaction is active".
This fix disables autoSave during the transaction to avoid this. Of course, autoSave is still called after the transaction, when calling commit.

There was another PR for this issue (#9101) but this fix is not working when saving the database inside a transaction. If we call a find on the repository, typeorm releases the connection and it's calling autoSave. Waiting for the transaction to finish before saving was not working as we wait indefinitely.

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run format to apply prettier formatting
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change N/A
• Documentation has been updated to reflect this change N/A
• The new commits follow conventions explained in CONTRIBUTING.md

[pleerock]

shouldn't we throw an error if one tries to autoSave during active transaction? Imagine someone called autoSave but it didn't save anything if transaction was active, and user still thinks everything was saved.

[Obiwan1995]

I get your point though it would be a bit tricky to do it as this method is automatically called on query runner release if database has changed (code from SqljsQueryRunner.ts):

private async flush() {
    if (this.isDirty) {
            await this.driver.autoSave()
            this.isDirty = false
    }
}

async release(): Promise<void> {
    await this.flush()
    return super.release()
}

If we throw an error everytime this gets called within a transaction, we will end up with tons of errors as a simple update on the repository during a transaction will trigger it.

Is there a way to inform the user without throwing an error?

[pleerock]

okay, another question:

shouldn't we add the proposed check into commitTransaction override?:

    async commitTransaction(): Promise<void> {
        await super.commitTransaction()
        if (this.isTransactionActive === false)
            await this.flush()
    }

[Obiwan1995]

You want to add it there to ensure that the database is saved after the transaction?

Currently, COMMIT sets isDirty to true. Then, when the query runner is released after the query, it automatically flushes the database as there is no active transaction.

So, database is already saved after transaction but it would be easier to understand I admit.

[pleerock]

It makes sense to have it there I think. We flush after the transaction. And we do it only if transaction was committed.

[Obiwan1995]

Any news on merging this?
My team had to fork the repository because of this so it would be nice to have it in the next release.

[pleerock]

Thank you for contribution! 🎉