firebase / firebase/firebase-cpp-sdk

Desktop RTDB: Repo::HandleTransactionResponse collapses every non-datastale server error (incl. permission_denied) to kErrorUnknownError with an empty message

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

説明

### Environment

- Firebase C++ SDK 13.9.0 (desktop/Windows; the cited code is unchanged at the 13.11.0 tag)
- Identified while tracing FlutterFire `firebase_database` 12.4.6 on Windows; the defect below is independently confirmed from the pinned desktop C++ source.

### What happens

When the server rejects a transaction commit (e.g. a security-rules fence returns `permission_denied` on the wire), the transaction `Future` completes with:

- `error()` = `kErrorUnknownError` (10) — not `kErrorPermissionDenied` (8)
- `error_message()` = `""` (empty)

so callers cannot distinguish "rules denied this commit" from any other failure, and there is no message to log. Plain `SetValue()` writes on the same path correctly surface `kErrorPermissionDenied`, which makes the transaction behavior surprising.

### Where the information is lost (pinned to `3d7ce2a`, the 13.9.0 tag)

1. The wire layer maps the response status correctly and retains the server message. `HandlePutResponse()` converts the status and passes the response body to `TriggerResponse()`:
https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/connection/persistent_connection.cc#L955-L970
`TriggerResponse()` stores both values, and the adjacent status map includes `permission_denied` → `kErrorPermissionDenied`:
https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/connection/persistent_connection.cc#L1244-L1281
2. `Repo::HandleTransactionResponse()` then discards both fields for every **error response** other than `datastale`:
```cpp
for (auto& transaction : response->queue()) {
transaction->status = TransactionData::kStatusNeedsAbort;
transaction->abort_reason = kErrorUnknownError;
}
```
https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/core/repo.cc#L982-L1000
3. `RerunTransactionQueue()` finally completes the future with the no-message overload, so `error_message()` is empty:
```cpp
DataSnapshot snapshot(new DataSnapshotInternal(
database_, node, QuerySpec(transaction->path)));
transaction->ref_future->CompleteWithResult(transaction->future_handle,
abort_reason, snapshot);
```
https://github.com/firebase/firebase-cpp-sdk/blob/3d7ce2a584d0b8daf1374bc2534c6ea71fa7fd6c/database/src/desktop/core/repo.cc#L1003-L1138

### Expected

`abort_reason` should carry the mapped response code (8 for `permission_denied`, etc.) and the completion should pass the response's error message through, matching what the Android SDK reports for the same server rejection.

### Impact

Any desktop app implementing fenced/optimistic transactions (rules-enforced epochs, leases, counters) receives an undiagnosable `unknown`/empty failure for what is actually a well-defined rules denial. Downstream SDK wrappers (e.g. FlutterFire's Windows plugin) inherit the collapsed code, so the loss is user-visible in every binding built on this implementation.

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

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

調査の方向性

database/src/desktop/core/repo.cc の Repo::HandleTransactionResponse() と RerunTransactionQueue() から始め、次に database/src/desktop/connection/persistent_connection.cc のレスポンスのステータスとメッセージのフローを比較します。トランザクションの失敗が、permission_denied を含め、マッピングされたサーバーエラーコードとメッセージを保持し、空のメッセージを伴う kErrorUnknownError として完了しないことを確認できれば完了です。

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

評価

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

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

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