firebase / firebase/firebase-cpp-sdk

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

オープン 初心者向け
#1,905 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
C++
スター
326
フォーク
137
平均マージ
3日 9時間
マージ済み PR(30日)
5

説明

### 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.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
cpp
領域
database, desktop
issue の種類
バグ
難易度
2/5
見積もり時間
1〜3時間
活発さ
静か
明瞭さ
明確に書かれている
初心者へのやさしさ
72/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。