Skip to content

Preserve completed Apple upload acknowledgements during cancellation - #1556

Merged
bmehta001 merged 1 commit into
microsoft:mainfrom
bmehta001:fix/apple-completed-upload-cancellation
Nov 5, 2026
Merged

bmehta001 merged 1 commit into
microsoft:mainfrom
bmehta001:fix/apple-completed-upload-cancellation

Conversation

@bmehta001

@bmehta001 bmehta001 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reduce avoidable duplicate uploads when cancellation races with an already completed Apple HTTP response. Related to #1542; this is a partial mitigation, not a resolution of the full issue.

  • Preserve the existing HTTP 200 result, headers and body only when NSURLSession completed without error and response headers passed the existing size limit.
  • Keep pending cancellation authoritative for transport errors (including cancellation and connection loss), missing HTTP responses, oversized headers and non-200 responses. The streaming delegate's response-body cap remains unchanged.
  • Add deterministic tests against the actual response handler using a helper compiled only in Apple unit-test builds. No new shipping state, configuration, public API or client-side deduplication machinery.

Validation

  • Changed C++ ranges pass the repository clang-format configuration; Objective-C++ ranges use the same configuration with its language set to ObjC because the checked-in configuration declares only Cpp. Whitespace checks pass.
  • Windows / Visual Studio 2026: UnitTests build passed; all 38 tests selected by HttpClientTests.*:HttpResponseDecoderTests.* passed.
  • WSL / Ubuntu / GCC 13: UnitTests build passed with the checked-out Azure Monitor module enabled; all 21 tests selected by the same filter passed. The initial local AZMON=OFF configuration could not link the module's auto-discovered tests; enabling the module resolved this without source changes or test exclusions.
  • Apple CI: the macos-14 Release / iPhone 15 simulator job passed. Its actual logs confirm all three new tests ran and passed: CompletedAppleResponseSurvivesPendingCancellation, PendingAppleCancellationDoesNotAcceptUnconfirmedResponses, and AppleResponseValidationWithoutCancellationIsUnchanged. Unit and functional XCTest suites also passed in that job: https://github.com/microsoft/cpp_client_telemetry/actions/runs/41979913757/job/127087389622
  • No negative test filters or new skipped tests. The new tests are intentionally Apple-transport-only and are not exercised by Windows/Linux runs. The remaining CI matrix is still running; physical-device lifecycle behavior was not tested.

Scope

This does not prevent duplicate ingestion after a lost acknowledgement or abrupt termination. Retrying ambiguous uploads remains intentional to avoid losing telemetry. Receiver-side idempotency/deduplication is still needed for that broader problem, which remains open.

Let an error-free, header-bounded HTTP 200 completion retain its response
when cancellation is pending. Keep interrupted, failed, oversized and
non-200 requests on the existing cancellation path; this does not solve
lost acknowledgements or promise exactly-once delivery.

Exercise the real response handler deterministically through a helper
compiled only in Apple unit-test builds, without new shipping state.

Files changed:
- lib/http/HttpClient_Apple.mm
- tests/unittests/HttpClientTests.cpp
- tests/unittests/CMakeLists.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f83491fd-5a6e-40af-b973-e83a3d7dad75
@bmehta001 bmehta001 self-assigned this Nov 5, 2026
@bmehta001
bmehta001 marked this pull request as ready for review November 5, 2026 06:26
@bmehta001
bmehta001 requested a review from a team as a code owner November 5, 2026 06:26
@bmehta001
bmehta001 enabled auto-merge (squash) November 5, 2026 06:29
@bmehta001
bmehta001 merged commit bb81f49 into microsoft:main Nov 5, 2026
94 checks passed
@bmehta001
bmehta001 deleted the fix/apple-completed-upload-cancellation branch November 5, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants