Map cancelled request body uploads to OperationCanceledException in AndroidMessageHandler

[Copilot]

When a caller cancels the CancellationToken while AndroidMessageHandler is still uploading a request body (e.g. a PUT whose body the server delays reading), the cancellation surfaced as WebException/IOException ("Socket closed") instead of an OperationCanceledException subtype, diverging from SocketsHttpHandler.

The cancellation callback disconnects the socket, so the in-flight CopyToAsync to httpConnection.OutputStream throws Java.IO.IOException, which was then wrapped into a WebException by DoSendAsync. The response-body read path already mapped these to cancellation; the request-upload path did not.

Changes

• WriteRequestContentToOutput: catch transport exceptions thrown during the body copy when cancellation has been requested, and rethrow as System.OperationCanceledException tied to the caller's token.
• ShouldMapToCancellation: hoisted from the nested CancellationAwareResponseStream to the enclosing AndroidMessageHandler so the upload and response-read paths share one mapping rule (no change to the existing response-read behavior).
• Test: RequestBodyUploadCancellationIsPrompt stalls a raw-socket server after reading the request headers, streams a large body in chunks until the send buffer blocks, cancels mid-upload, and asserts a prompt OperationCanceledException.

try {
    await stream.CopyToAsync (httpConnection.OutputStream!, 4096, cancellationToken).ConfigureAwait (false);
} catch (Exception ex) when (ShouldMapToCancellation (ex, cancellationToken)) {
    throw new System.OperationCanceledException ("Request body upload was canceled.", ex, cancellationToken);
}

[Copilot]

Pull request overview

This PR aligns Xamarin.Android.Net.AndroidMessageHandler cancellation behavior with SocketsHttpHandler by ensuring that cancellations during request body uploads surface as an OperationCanceledException (instead of transport-level WebException/IOException), matching the already-existing behavior for canceled response-body reads.

Changes:

• Hoists ShouldMapToCancellation() to AndroidMessageHandler so both upload and response-read paths share the same cancellation mapping logic.
• Updates WriteRequestContentToOutput() to map transport exceptions during request-body copy to an OperationCanceledException when cancellation is requested.
• Adds an on-device test that stalls request-body reads server-side, cancels mid-upload, and asserts prompt cancellation.

Reviewed changes

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

File	Description
src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs	Maps request-body upload transport failures (triggered by cancellation disconnect) to OperationCanceledException; shares cancellation-mapping helper with response-read path.
tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerCancellationTests.cs	Adds a stalled-request upload-cancellation test and supporting server/content helpers.

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Android PR Review

Verdict: 👍 Looks correct — a few optional 💡 nits (final sign-off pending CI).

I reviewed the change independently before reading the description, then reconciled. The fix is well-targeted and addresses the root cause.

What the change does (verified)

WriteRequestContentToOutput uploads the request body via stream.CopyToAsync (httpConnection.OutputStream!, ..., cancellationToken). When the caller cancels mid-upload, the cancellationToken.Register callback in DoProcessRequest disconnects the socket, so the in-flight write throws Java.IO.IOException ("Socket closed"). Previously that exception bubbled up to DoSendAsync's catch (Java.IO.IOException ...) and was re-wrapped into a WebException — diverging from SocketsHttpHandler. The new catch ... when (ShouldMapToCancellation (ex, cancellationToken)) intercepts it first and rethrows OperationCanceledException tied to the token, so it propagates past the WebException mappers unchanged. ✅ Root cause, not symptom.

• ✅ Hoisting ShouldMapToCancellation from the nested CancellationAwareResponseStream to AndroidMessageHandler is safe — it's static, and both existing call sites (response-read path) resolve to the enclosing type. No behavior change to the response path.
• ✅ Consistent with the existing response-read mapping ("Response body read was canceled.") — same exception type and shape.
• ✅ Also improves the HttpClient.Timeout path: a timeout-triggered cancellation now surfaces as cancellation rather than WebException.
• ✅ Includes a focused regression test (RequestBodyUploadCancellationIsPrompt) that stalls a raw-socket server after the headers, fills the send buffer with a 16 MB body, cancels mid-upload, and asserts a prompt OperationCanceledException. The test fails on the old code and passes on the new code.

Findings

Severity	Count
❌ error	0
⚠️ warning	0
💡 suggestion	3

All three are optional, test-only polish (posted inline):

1. 💡 serverTask is stored but never observed in Dispose () — unlike the sibling StalledResponseServer.Stop ().
2. 💡 StalledRequestServer duplicates much of StalledResponseServer's TCS scaffolding.
3. 💡 ZeroStream.ReadAsync ignores its cancellationToken (harmless, contract-only).

None block merge.

CI

The Azure DevOps dotnet-android pipeline is still in progress for the latest commit (all completed legs are green so far; the new test runs under the MSBuild+Emulator legs). This is a COMMENT, not an approval — please confirm the pipeline goes green (especially the emulator leg exercising AndroidMessageHandlerCancellation) before merging.

Nice, surgical fix with a real regression test. 👍

Generated by Android PR Reviewer for issue #11683 · 545 AIC · ⌖ 50.9 AIC · ⊞ 37.8K
_{Comment /review to run again}

[github-actions]

🤖 💡 Code organization — serverTask is stored but never observed. Dispose () releases the body and stops the listener, but unlike the sibling StalledResponseServer.Stop () (which calls ObserveServerTask ().GetAwaiter ().GetResult ()), it never awaits serverTask. As written the field only roots the task, and any unexpected server-side fault raised after bodyStarted completes is silently swallowed by StallRequestBody's catch. Consider observing serverTask in Dispose () for parity and to surface real failures.

Rule: Consistency with sibling helpers / observe background tasks

[github-actions]

🤖 💡 Patterns — StalledRequestServer largely mirrors StalledResponseServer: the same bodyStarted/releaseBody TaskCompletionSource setup, Port, BodyStartedTask, a release method, and the identical "fault before body started → TrySetException, else log" handling. Consider extracting the shared stall/signal scaffolding into a small base helper so the two servers can't drift. Optional — skip if it would over-complicate the harness.

Rule: Centralize duplicate algorithms (Postmortem #54)

[github-actions]

🤖 💡 Testing — ZeroStream.ReadAsync ignores its cancellationToken. Harmless here because reads complete synchronously, but for Stream contract correctness consider honoring it, e.g. if (cancellationToken.IsCancellationRequested) return ValueTask.FromCanceled<int> (cancellationToken);. Low priority.

Rule: Honor the token (csharp-rules: Async & Cancellation)