firebase / firebase/firebase-cpp-sdk
Desktop RTDB: Repo::HandleTransactionResponse collapses every non-datastale server error (incl. permission_denied) to kErrorUnknownError with an empty message
- 主要言語
- 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