diff --git a/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift b/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift index 2608bd5cc05..35295425189 100644 --- a/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift +++ b/Firestore/Swift/Tests/Integration/QueryIntegrationTests.swift @@ -166,6 +166,17 @@ class QueryIntegrationTests: FSTIntegrationTestCase { matchesResult: ["doc2"]) } + func testFailedWriteBatch() throws { + let batch = db.batch() + .updateData(["a": 1, "b": 0], forDocument: documentRef()) + let a = expectation(description: "wait for mutation") + batch.commit { [weak self] err in + XCTAssert(err != nil) + a.fulfill() + } + awaitExpectation(a) + } + func testOrQueriesWithIn() throws { let collRef = collectionRef( withDocuments: ["doc1": ["a": 1, "b": 0], diff --git a/Firestore/core/src/core/firestore_client.cc b/Firestore/core/src/core/firestore_client.cc index 6282cfb2ae0..02ba5b7499f 100644 --- a/Firestore/core/src/core/firestore_client.cc +++ b/Firestore/core/src/core/firestore_client.cc @@ -523,6 +523,8 @@ void FirestoreClient::WriteMutations(std::vector&& mutations, if (mutations.empty()) { if (callback) { user_executor_->Execute([=] { callback(Status::OK()); }); + } else { + LOG_WARN("Skipping callback due to empty mutations"); } } else { sync_engine_->WriteMutations( @@ -530,6 +532,8 @@ void FirestoreClient::WriteMutations(std::vector&& mutations, // Dispatch the result back onto the user dispatch queue. if (callback) { user_executor_->Execute([=] { callback(std::move(error)); }); + } else { + LOG_WARN("Skipping callback due to null call back"); } }); } diff --git a/Firestore/core/src/core/sync_engine.cc b/Firestore/core/src/core/sync_engine.cc index 77223cb1fed..6546d499604 100644 --- a/Firestore/core/src/core/sync_engine.cc +++ b/Firestore/core/src/core/sync_engine.cc @@ -408,9 +408,9 @@ void SyncEngine::HandleRejectedWrite( DocumentMap changes = local_store_->RejectBatch(batch_id); - if (!changes.empty() && ErrorIsInteresting(error)) { + if (!changes.empty()) { const DocumentKey& min_key = changes.min()->first; - LOG_WARN("Write at %s failed: %s", min_key.ToString(), + LOG_WARN("Write batch %s at %s failed: %s", batch_id, min_key.ToString(), error.error_message()); } @@ -469,6 +469,10 @@ void SyncEngine::NotifyUser(BatchId batch_id, Status status) { // NOTE: Mutations restored from persistence won't have callbacks, so // it's okay for this (or the callback below) to not exist. if (it == mutation_callbacks_.end()) { + if (!status.ok()) { + LOG_WARN("Could not find mutation callback queue for user %s", + current_user_.uid()); + } return; } @@ -477,6 +481,9 @@ void SyncEngine::NotifyUser(BatchId batch_id, Status status) { if (callback_it != callbacks.end()) { callback_it->second(std::move(status)); callbacks.erase(callback_it); + } else if (!status.ok()) { + LOG_WARN("Could not find mutation callback for user %s and batch %s", + current_user_.uid(), batch_id); } }