firebase / firebase/firebase-cpp-sdk

Desktop RTDB: a transaction callback abort on a rerun completes the Future with kErrorNone

Aperta Adatta ai principianti
#1,905 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
C++
Stelle
326
Fork
137
Merge medio
3g 9h
PR unite (30g)
5

Descrizione

### Environment

- Firebase C++ SDK 13.9.0 (desktop; the cited code is unchanged at the 13.11.0 tag)

### Summary

On desktop, if the `DoTransaction` callback aborts on the **first** invocation, the future completes with `kErrorWriteCanceled` (11) — see `Repo::StartTransaction`:
https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/core/repo.cc#L659-L729

But if the abort happens on a **rerun** invocation (after a `datastale` response forces the transaction to run again), the abort reason is taken from a local `error` variable that was initialized to `kErrorNone`:

- `RerunTransactionQueue()` initializes `Error error = kErrorNone` at line 1067. If the rerun callback returns abort, line 1095 assigns that unchanged zero value to the function-local `abort_reason`.
- Lines 1111–1112 copy `abort_reason` and the current input into `FutureToComplete`. No later assignment changes the reason: line 1129 references the queued value, and lines 1131–1134 complete the Future with `CompleteWithResult(..., abort_reason, snapshot)`.

https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/core/repo.cc#L1062-L1138

The result: the Future completes with `error() == kErrorNone` and a snapshot — indistinguishable from a successful commit — for a transaction that was **aborted and never committed**.

### Expected

A rerun-invocation abort should complete with the same abort code as a first-invocation abort (or, better, with `kErrorTransactionAbortedByUser` to match the mobile SDKs), never with `kErrorNone`.

### Impact

Consumers that branch on the future's error to decide "did my transaction commit?" will believe an aborted transaction succeeded whenever the abort happened after a `datastale` rerun — e.g. optimistic/fenced updates that abort once fresh data shows another writer won. In the FlutterFire Windows plugin this surfaces as `TransactionResult(committed: true)` for an aborted transaction.

### Related

Filed alongside a second desktop transaction-error report: `Repo::HandleTransactionResponse` collapses every non-`datastale` server error (including `permission_denied`) to `kErrorUnknownError` with an empty message. Both were found while debugging the same Windows application.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

In database/src/desktop/core/repo.cc, read StartTransaction and RerunTransactionQueue first, tracing abort_reason, the local error, FutureToComplete, and CompleteWithResult. Reproduce or add coverage for a datastale-triggered rerun whose callback aborts; done means the Future reports a non-success abort code rather than kErrorNone and no commit is reported.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
cpp
Ambito
database, desktop
Tipo di issue
Bug
Difficoltà
2/5
Tempo stimato
1-3 ore
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
72/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.