Cosmos: flow transactional batch failures through the execution strategy so they can be retried

[Copilot]

Cosmos transactional batch failures returned a result object from ProcessBatchResponse rather than throwing, so the failure surfaced as a DbUpdateException outside any execution-strategy scope and could never be retried by an ExecutionStrategy (ShouldRetryOn was never called).

Changes

• Throw wrapped exception on batch failure — CosmosClientWrapper.ProcessBatchResponse now computes the errored entries (those whose per-item status code matches the batch-level error code), wraps the CosmosException into a DbUpdateConcurrencyException (PreconditionFailed) or DbUpdateException (Conflict / other), and throws it directly. CosmosTransactionalBatchResult and CosmosTransactionalBatchException are removed.
• Execute batches once in the client — ExecuteTransactionalBatchAsync no longer wraps itself in the execution strategy; it runs once and returns Task (interface updated).
• Drive retries from SaveChangesAsync — CosmosDatabaseWrapper takes an injected IExecutionStrategy and wraps the whole set of batches in a single ExecuteAsync. A BatchExecutionState.CommittedOperations counter lets a retry skip already-committed operations while preserving the original ordering of single-entry saves interleaved with transactional batches. The execution strategy unwraps DbUpdateException via ExecutionStrategy.CallOnWrappedException to reach the inner CosmosException before calling ShouldRetryOn, so transient failures are correctly identified and retried.
• Suppressed concurrency consistency — when a DbUpdateConcurrencyException from a batch is suppressed via OptimisticConcurrencyExceptionAsync, RowsAffected is not incremented (matching SaveAsync single-entry behavior), while CommittedOperations still advances so a subsequent retry skips the already-processed operation.

Because a transactional batch is atomic, a failed batch commits nothing and is retried in place; deterministic transaction serialization across attempts makes index-based skipping of committed operations valid.

Result

The repro from the issue now behaves as expected — a transient batch failure reaches the strategy and is retried:

protected override bool ShouldRetryOn(Exception exception)
{
    Console.WriteLine($"Exception: {exception.Message}"); // now written
    return true;
}

Two functional tests verify the behavior:

• SaveChanges_transactional_batch_failure_flows_through_execution_strategy — a conflicting batch reaches ShouldRetryOn and yields RetryLimitExceededException, with nothing committed.
• SaveChanges_suppressed_concurrency_exception_in_transactional_batch_reports_zero_rows_affected — a stale-ETag batch update suppressed by an interceptor returns 0 rows affected and leaves the store unchanged.

[Copilot]

Pull request overview

This PR fixes Cosmos transactional batch failures bypassing the execution strategy retry pipeline by throwing on batch failure (instead of returning a “failed result”) and moving the execution-strategy scope to wrap the full SaveChanges batch sequence, allowing transient failures to reach ShouldRetryOn and be retried.

Changes:

• Replace the transactional batch “result” return pattern with a CosmosTransactionalBatchException thrown from ProcessBatchResponse.
• Change ICosmosClientWrapper.ExecuteTransactionalBatchAsync to return Task and execute once (no per-batch execution strategy wrapping in the client).
• Wrap the full set of transactional batches in a single execution strategy invocation inside CosmosDatabaseWrapper, with retry skipping of already-committed operations, and add a functional test verifying the exception reaches the execution strategy.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/EFCore.Cosmos.FunctionalTests/CosmosTransactionalBatchTest.cs	Adds a functional test asserting transactional batch failures flow through the execution strategy and nothing is committed.
src/EFCore.Cosmos/Storage/Internal/ICosmosClientWrapper.cs	Updates the transactional batch API to return Task rather than a result object.
src/EFCore.Cosmos/Storage/Internal/CosmosTransactionalBatchException.cs	Introduces an internal exception type carrying errored entries and the underlying CosmosException.
src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs	Moves execution strategy retry scope to encompass the entire batch set and adds committed-operation skipping across retries.
src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs	Executes transactional batches once and throws CosmosTransactionalBatchException on failure (instead of returning a failure result).

[AndriySvyryd]

@copilot Make sure there's no duplication between this code and WrapUpdateException, also check other modified code for opportunities to refactor

[Copilot]

Extracted the CosmosException → DbUpdateException switch to a new public static CosmosClientWrapper.WrapUpdateException(Exception, string id, IReadOnlyList<IUpdateEntry>) method. ProcessBatchResponse now calls it directly, and CosmosDatabaseWrapper.WrapUpdateException computes the document id and delegates to the same method — removing the duplicate switch. Also cleaned up using System.Net (no longer needed in CosmosDatabaseWrapper) and replaced transaction.Entries.First().Entry.Context with allEntries[0].Context since allEntries was already available.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.