simolus3 / drift

Drift is an easy to use, reactive, typesafe persistence library for Dart & Flutter.
https://drift.simonbinder.eu/
MIT License
2.54k stars 360 forks source link

Web: Error: cannot rollback - no transaction is active #1806

Open stx opened 2 years ago

stx commented 2 years ago

Seen on Firefox and Chrome on production. Latest everything. Two different stacktraces, one with runMigrations listed and one without. No isolate.

Database open:

  return Database(
    WebDatabase.withStorage(
      await DriftWebStorage.indexedDbIfSupported('db'),
    ),
  );

Migration strategy:

          try {
            for (TableInfo<Table, dynamic> table in allTables) {
              await m.deleteTable(table.actualTableName);
              await m.createTable(table);
            }
          } catch (e, stackTrace) {
            handleDriftException(e, stackTrace);
          }
Error: cannot rollback - no transaction is active
  at detectOperatingSystem(main.dart.js:198:1)
  at _Random(../../../../../flutter/bin/cache/flutter_web_sdk/lib/_engine/engine/browser_detection.dart:198:1)
  at _WebDelegate.open(org-dartlang-sdk:///lib/_internal/js_runtime/lib/math_patch.dart:173:7)
  at DelegatedDatabase._runMigrations(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/web/web_db.dart:104:11)
  at _TransactionExecutor.rollback(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/runtime/executor/helpers/engines.dart:309:11)
  at Future.sync(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/runtime/executor/helpers/engines.dart:247:34)
  at _IndexedDbStorage.restore(org-dartlang-sdk:///lib/async/future_impl.dart:279:28)
  at _IndexedDbStorage.restore(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/web/storage.dart:227:48)
  at _rootRunBinary(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/web/storage.dart:231:5)
  at Object.bfe(org-dartlang-sdk:///lib/async/zone.dart:1462:1)
Error: cannot rollback - no transaction is active
  at detectOperatingSystem(main.dart.js:198:1)
  at JsObject.callMethod(../../../../../flutter/bin/cache/flutter_web_sdk/lib/_engine/engine/browser_detection.dart:198:1)
  at SqlJsDatabase.runWithArgs(org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_patch.dart:217:12)
  at _BaseExecutor.runCustom.<anonymous function>(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/web/sql_js.dart:99:7)
  at _BaseExecutor._synchronized.<anonymous function>(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/web/web_db.dart:147:9)
  at Future.sync(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/runtime/executor/helpers/engines.dart:51:16)
  at Lock.synchronized.callBlockAndComplete(org-dartlang-sdk:///lib/async/future.dart:296:20)
  at Lock.synchronized.<anonymous function>(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/utils/synchronized.dart:17:7)
  at _rootRunUnary(../../../../../flutter/.pub-cache/hosted/pub.dartlang.org/drift-1.5.0/lib/src/utils/synchronized.dart:21:35)
  at _rootRunUnary[function-entry$5](org-dartlang-sdk:///lib/async/zone.dart:1434:46)
  at _CustomZone.runUnary(org-dartlang-sdk:///lib/async/zone.dart:1432:3)
  at _FutureListener.handleValue(org-dartlang-sdk:///lib/async/zone.dart:1334:34)
  at _Future._propagateToListeners(org-dartlang-sdk:///lib/async/future_impl.dart:123:29)
  at _Future._completeWithValue(org-dartlang-sdk:///lib/async/future_impl.dart:795:13)
  at _Future._asyncCompleteWithValue.<anonymous function>(org-dartlang-sdk:///lib/async/future_impl.dart:601:5)
  at _rootRun(org-dartlang-sdk:///lib/async/future_impl.dart:639:7)
  at _rootRun[function-entry$4](org-dartlang-sdk:///lib/async/zone.dart:1426:12)
  at _CustomZone.run(org-dartlang-sdk:///lib/async/zone.dart:1417:3)
  at _CustomZone.runGuarded(org-dartlang-sdk:///lib/async/zone.dart:1327:34)
  at _CustomZone.bindCallbackGuarded.<anonymous function>(org-dartlang-sdk:///lib/async/zone.dart:1236:7)
  at _microtaskLoop(org-dartlang-sdk:///lib/async/zone.dart:1276:23)
  at _startMicrotaskLoop(org-dartlang-sdk:///lib/async/schedule_microtask.dart:40:12)
  at _AsyncRun._initializeScheduleImmediate.internalCallback(org-dartlang-sdk:///lib/async/schedule_microtask.dart:49:5)
  at invokeClosure(org-dartlang-sdk:///lib/_internal/js_runtime/lib/async_patch.dart:49:10)
  at convertDartClosureToJS(org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart:1833:14)
  at k6.<fn>(org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart:1865:1)
simolus3 commented 2 years ago

It's weird that this is, two times, happening with detectOperatingSystem in the stack trace. I also don't understand how SqlJsDatabase.runWithArgs would call that in JS, maybe it's a weird dart2js optimization...?

In handleDriftException, are you printing the original stack trace?

stx commented 2 years ago

@simolus3 handleDriftException was just Sentry.captureException(e, stackTrace: stackTrace); and this is what Sentry collected.

This is what we've expanded it to (no error reports collected with it yet):

void handleDriftException(Object e, StackTrace? stackTrace) {
  if (e is DriftRemoteException) {
    Sentry.captureException(e.remoteCause, stackTrace: e.remoteStackTrace);
  } else if (e is DriftWrappedException) {
    Sentry.captureException(e.cause, stackTrace: e.trace, hint: e.message);
  } else if (e is CouldNotRollBackException) {
    Sentry.captureException(e.cause, stackTrace: e.originalStackTrace, hint: e.exception);
    } else {
    Sentry.captureException(e, stackTrace: stackTrace);
  }
}
stx commented 2 years ago

With extra error handling added (from Android 11 w/isolate):

String: SqliteException(1): cannot rollback - no transaction is active
  Causing statement: ROLLBACK TRANSACTION
  File "database.dart", line 129, in DatabaseImpl.execute
  File "database.dart", line 204, in _VmDelegate._runWithArgs
  File "database.dart", line 214, in _VmDelegate.runCustom
  File "engines.dart", line 109, in _BaseExecutor.runCustom.<fn>
  File "engines.dart", line 51, in _BaseExecutor._synchronized.<fn>
  File "future.dart", line 296, in new Future.sync
  File "synchronized.dart", line 17, in Lock.synchronized.callBlockAndComplete
  File "server_impl.dart", line 226, in ServerImplementation._waitForTurn.<fn>
  File "<asynchronous suspension>"
  File "engines.dart", line 248, in _TransactionExecutor.rollback
  File "<asynchronous suspension>"
  File "server_impl.dart", line 197, in ServerImplementation._transactionControl
  File "<asynchronous suspension>"
  File "communication.dart", line 156, in DriftCommunication.setRequestHandler.<fn>.<fn>
simolus3 commented 2 years ago

Thanks for the additional info! You said no isolates, but there's a ServerImplementation in the stack trace. I assume you're using web workers then? Also how can there be a _VmDelegate on the web? That one uses dart:ffi, so this seems pretty weird.

stx commented 2 years ago

Thanks for the additional info! You said no isolates, but there's a ServerImplementation in the stack trace. I assume you're using web workers then? Also how can there be a _VmDelegate on the web? That one uses dart:ffi, so this seems pretty weird.

I went back through Sentry and it looks like it merged the issues by mistake. That trace was from Android 11, with an isolate.

simolus3 commented 2 years ago

Are you using something like INSERT OR ROLLBACK anywhere (or a trigger using RAISE(ROLLBACK))? Drift assumes that it has full control over transactions, so it can get tripped up by statements closing the transaction behind its back.

I haven't really considered constructs like OR ROLLBACK in the implementation to be honest. I think throwing a CouldNotRollBackException and ensuring that the database is in a valid state afterwards may be the best action here.

Judging from your stack trace however, it doesn't seem like you're getting a CouldNotRollBackException, which probably means that you're running into this catch block in drift. So what I think is happening here is:

  1. The transaction body goes through without throwing an exception.
  2. We then call COMMIT;, which for some reason throws an exception.
  3. To handle the call, we call ROLLBACK; which then fails because there's no transaction active?

So far I don't understand how step 3 could fail. To my knowledge (I poked around with a bit of testing), transactions stay active if a COMMIT fails. If you have more insights on how this might happen I'd love to write a proper test against sqlite3 for this, so far I'm using mocks.

In 5d6933d9b7e73d24c35f6ce5272b62dc65865373, I changed the implementation to throw a more descriptive error that also explains why the rollback was attempted in the first place. That won't solve your problem, but hopefully allow us to figure out what's going on here.

stx commented 2 years ago

Thanks for taking the time and addressing this (and everything else) so thoughtfully. It really shows and we appreciate it.

To answer your question, nope, no rollbacks used anywhere. It’s a pretty vanilla SQL implementation. We do use batch inserts and deletes. Maybe a result of that?

stx commented 2 years ago

There are situations where batches might run simultaneously. Possibly a result of that?

simolus3 commented 2 years ago

It could be because of that, but it's also happening on the web where simultaneous edits to the same database connection aren't possible (even if you had the app open in multiple tabs they'd use independent connections as far as sqlite3 is concerned). Let's see if the updated exception reporting to sentry or the more detailed rollback handling in drift will provide more insights here.

LeFrosch commented 1 year ago

I am currently facing a similar error and I might have an idea what causes this error. In my case there a multiple queries that use a transaction and they can be started simultaneously. I think somehow the engine gets confused when a transaction is started before another transaction has finished. When I synchronize all queries using a Mutex, the error seams to be fixed.