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

[NH-Mason]

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:

firebase-cpp-sdk/database/src/desktop/core/repo.cc

Lines 659 to 729 in 3d7ce2a

	void Repo::StartTransaction(const Path& path,
	DoTransactionWithContext transaction_function,
	void* context, void (*delete_context)(void*),
	bool trigger_local_events,
	ReferenceCountedFutureImpl* api,
	SafeFutureHandle<DataSnapshot> handle) {
	// Make sure we're listening on this node.
	// Note: we can't do this asynchronously. To preserve event ordering, it has
	// to be done in this block. This is ok, this block is guaranteed to be our
	// own event loop
	DatabaseReferenceInternal* ref_impl =
	new DatabaseReferenceInternal(database_, path);
	DatabaseReference watch_ref(ref_impl);
	std::unique_ptr<NoopListener> listener = std::make_unique<NoopListener>();
	NoopListener* listener_ptr = listener.get();
	QuerySpec query_spec(path);
	AddEventCallback(std::make_unique<ValueEventRegistration>(
	database_, listener_ptr, query_spec));
	TransactionDataPtr transaction_data = std::make_shared<TransactionData>(
	handle, api, query_spec.path, transaction_function, context,
	delete_context, trigger_local_events, std::move(listener));
	// Run transaction initially.
	Variant current_state = GetLatestState(path);
	transaction_data->current_input_snapshot = current_state;
	MutableDataInternal* mutable_data_impl =
	new MutableDataInternal(database_, current_state);
	MutableData mutable_current(mutable_data_impl);
	TransactionResult result = transaction_function(&mutable_current, context);
	if (result != kTransactionResultSuccess) {
	// Abort the transaction.
	transaction_data->current_output_snapshot_raw = Variant::Null();
	transaction_data->current_output_snapshot_resolved = Variant::Null();
	transaction_data->status = TransactionData::kStatusNeedsAbort;
	transaction_data->ref_future->Complete(transaction_data->future_handle,
	kErrorWriteCanceled);
	// If there was an error, the listener must be removed to prevent calls to
	// it in case the listener is destroyed.
	RemoveEventCallback(listener_ptr, query_spec);
	} else {
	// Mark as run and add to our queue.
	transaction_data->status = TransactionData::kStatusRun;
	auto* queue_node = transaction_queue_tree_.GetOrMakeSubtree(path);
	if (!queue_node->value().has_value()) {
	queue_node->set_value(std::vector<TransactionDataPtr>());
	}
	queue_node->value()->push_back(transaction_data);
	Variant server_values = GenerateServerValues(server_time_offset_);
	const Variant* new_node_unresolved = mutable_data_impl->GetNode();
	Variant new_node_resolved =
	ResolveDeferredValueSnapshot(*new_node_unresolved, server_values);
	transaction_data->current_output_snapshot_raw = *new_node_unresolved;
	transaction_data->current_output_snapshot_resolved = new_node_resolved;
	transaction_data->current_write_id = GetNextWriteId();
	std::vector<Event> events = server_sync_tree_->ApplyUserOverwrite(
	path, *new_node_unresolved, new_node_resolved,
	transaction_data->current_write_id,
	trigger_local_events ? kOverwriteVisible : kOverwriteInvisible,
	kDoNotPersist);
	PostEvents(events);
	SendAllReadyTransactions();
	}
	}

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

firebase-cpp-sdk/database/src/desktop/core/repo.cc

Lines 1062 to 1138 in 3d7ce2a

	transaction->current_input_snapshot = current_input;
	MutableDataInternal* mutable_data_impl =
	new MutableDataInternal(database_, current_input);
	MutableData mutable_data(mutable_data_impl);
	Error error = kErrorNone;
	TransactionResult result = transaction->transaction_function(
	&mutable_data, transaction->context);
	if (result == kTransactionResultSuccess) {
	WriteId old_write_id = transaction->current_write_id;
	Variant server_values = GenerateServerValues(server_time_offset_);
	Variant* new_data_node = mutable_data_impl->GetNode();
	Variant new_node_resolved =
	ResolveDeferredValueSnapshot(*new_data_node, server_values);
	transaction->current_output_snapshot_raw = *new_data_node;
	transaction->current_output_snapshot_resolved = new_node_resolved;
	transaction->current_write_id = GetNextWriteId();
	sets_to_ignore.push_back(old_write_id);
	Extend(&events,
	server_sync_tree_->ApplyUserOverwrite(
	transaction->path, *new_data_node, new_node_resolved,
	transaction->current_write_id,
	transaction->trigger_local_events ? kOverwriteVisible
	: kOverwriteInvisible,
	kPersist));
	Extend(&events, server_sync_tree_->AckUserWrite(
	old_write_id, kAckRevert, kDoNotPersist,
	server_time_offset_));
	} else {
	abort_transaction = true;
	abort_reason = error;
	Extend(&events, server_sync_tree_->AckUserWrite(
	transaction->current_write_id, kAckRevert,
	kDoNotPersist, server_time_offset_));
	}
	}
	}
	PostEvents(events);
	if (abort_transaction) {
	transaction->status = TransactionData::kStatusComplete;
	DatabaseReferenceInternal* database_ref_impl =
	new DatabaseReferenceInternal(database_, path);
	DatabaseReference ref(database_ref_impl);
	futures_to_complete.push_back(FutureToComplete(
	transaction, abort_reason, transaction->current_input_snapshot));
	// Removing a callback can trigger pruning which can muck with
	// merged_data/visible_data (as it prunes data). So defer removing the
	// callback until later.
	Repo::scheduler().Schedule(NewCallback(
	[](Repo* repo, TransactionDataPtr transaction) {
	repo->RemoveEventCallback(transaction->outstanding_listener.get(),
	QuerySpec(transaction->path));
	},
	this, transaction));
	}
	}
	PruneCompletedTransactions(&transaction_queue_tree_);
	for (auto& future_to_complete : futures_to_complete) {
	TransactionDataPtr& transaction = future_to_complete.transaction;
	Error& abort_reason = future_to_complete.abort_reason;
	Variant& node = future_to_complete.node;
	DataSnapshot snapshot(new DataSnapshotInternal(
	database_, node, QuerySpec(transaction->path)));
	transaction->ref_future->CompleteWithResult(transaction->future_handle,
	abort_reason, snapshot);
	}
	SendAllReadyTransactions();
	}

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.

[NH-Mason]

Related: #1904 (non-datastale server errors collapse to kErrorUnknownError with an empty message) and firebase/flutterfire#18549 (Windows plugin surface: initial-invocation aborts throw; a rerun abort hitting this defect instead reports committed: true).

[NH-Mason]

Clarification on verification status: this report's rerun-abort result is source-analysis-derived at the pinned 13.9.0 commit 3d7ce2a; [NEEDS VERIFICATION: a runtime capture that forces a datastale rerun and then aborts] does not yet exist. Our retained Windows runtime capture covers the adjacent, different initial-invocation abort path and shows the resulting FirebaseException(code: 'unknown', message: ''); the pinned source shows that path completes with native error 11 (kErrorWriteCanceled) and no message. For #1905, the source-derived chain is that RerunTransactionQueue() initializes error to kErrorNone, copies that unchanged value into the callback-abort reason, and ultimately calls CompleteWithResult with error 0. We have now re-verified those statements line by line at 3d7ce2a. We also compared the entire database/src/desktop/core/repo.cc file at Firebase C++ SDK 13.11.0 and found it byte-identical, so the analyzed control flow is unchanged there. A controlled runtime capture would strengthen the report, but the source-level defect remains: the aborted rerun path can reach Future completion with the SDK's success code.