Skip to content

loadFile masks statement errors by rolling back twice #308

Description

@chrispader

Summary

loadFile() rolls back twice after a statement failure. The second ROLLBACK fails with cannot rollback - no transaction is active, masking the actual import failure and the library's intended CouldNotLoadFile error.

Verified on main at ad8b835 (v9.7.0).

Code evidence

In importSqlFile.cpp#L20-L41:

  1. A per-line NitroSQLiteException catch executes ROLLBACK at line 28.
  2. It throws CouldNotLoadFile at line 30.
  3. The outer catch (...) catches that exception and executes ROLLBACK again at line 40.
  4. SQLite reports cannot rollback - no transaction is active; that new SqlExecutionError escapes before the intended UnknownError at line 41 can be thrown.

The same outer catch also replaces useful BEGIN/COMMIT failures with a generic message, and a rollback failure can replace any primary error.

Smallest reproducer

Create a SQL file with one valid command followed by invalid SQL:

CREATE TABLE imported(id INTEGER PRIMARY KEY);
THIS IS INVALID SQL;

Then:

const db = open({ name: 'load-file.sqlite' })

await expect(db.loadFileAsync(path)).rejects.toThrow()

Expected:

  • The transaction is rolled back exactly once.
  • The error identifies loadFile, the source file, and ideally the failing command/line.
  • imported does not exist.

Observed from the current control flow:

  • The transaction is rolled back.
  • The intended CouldNotLoadFile(..., "Transaction was rolled back") is caught internally.
  • A second rollback fails and surfaces cannot rollback - no transaction is active, masking the import error.

The rollback behavior itself is reproducible with BEGIN; ROLLBACK; ROLLBACK;, where SQLite rejects the second rollback.

Impact

Import failures lose their actionable error and source context. Applications cannot reliably distinguish malformed import files from transaction-state errors, making migrations and recovery diagnostics substantially harder.

Acceptance criteria

  • Give one scope sole ownership of transaction finalization; execute at most one rollback per failed import transaction.
  • Track whether BEGIN succeeded and whether the transaction is still active before attempting rollback.
  • Preserve the primary statement/BEGIN/COMMIT error, including file and failing line/command context.
  • If rollback also fails, retain both errors without masking the primary cause.
  • Close the input file through RAII rather than repeated manual close paths.

Regression-test target

A native or Harness test that imports a file containing a valid command followed by invalid SQL and asserts:

  1. The promise rejects with the original import/SQL context, not cannot rollback - no transaction is active.
  2. The valid command was rolled back.
  3. The same connection remains usable afterward.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions