firebase / firebase/firebase-cpp-sdk
Desktop RTDB: Repo::HandleTransactionResponse collapses every non-datastale server error (incl. permission_denied) to kErrorUnknownError with an empty message
- Lenguaje dominante
- C++
- Estrellas
- 326
- Forks
- 137
- Merge medio
- 3 d 9 h
- PR fusionados (30 d)
- 5
Descripción
### 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.
Guía de contribución
Línea de trabajo
Start in database/src/desktop/core/repo.cc at Repo::HandleTransactionResponse() and RerunTransactionQueue(), then compare the response status and message flow in database/src/desktop/connection/persistent_connection.cc. Done means transaction failures preserve the mapped server error code and message, including permission_denied, instead of completing as kErrorUnknownError with an empty message.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- cpp
- Área
- databases
- Tipo de issue
- Error
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Estado de actividad
- Tranquilo
- Claridad
- Bien especificado
- Aptitud para principiantes
- 70/100