[NSUrlSessionHandler] Surface cancelled requests with a better exception. Fixes #25667.#25699
Draft
rolfbjarne wants to merge 7 commits into
Draft
[NSUrlSessionHandler] Surface cancelled requests with a better exception. Fixes #25667.#25699rolfbjarne wants to merge 7 commits into
rolfbjarne wants to merge 7 commits into
Conversation
…ion. Fixes #25667. Add a test: that sets up a stalling HTTP server, reads the first byte successfully, then cancels the second ReadAsync via a caller-owned CancellationToken. TODO: * Implement the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Localhost server access should be reliable, so there's no need to paper over failures with IgnoreInCI/Inconclusive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
With Content-Length set, HttpListener may buffer the output until all bytes are written. Switching to chunked transfer encoding ensures the first byte reaches the client immediately, so the first ReadAsync completes and the test can proceed to the cancellation scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stream-readasync-cancellation-test
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
HttpListener buffers output even with chunked encoding, preventing the first byte from reaching the client in time. Using a raw TcpListener with NoDelay lets us write HTTP response bytes directly to the socket, ensuring the first chunk is delivered immediately so the test can proceed to the cancellation scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Three changes to make the test work reliably: 1. Wait for the complete HTTP request (\r\n\r\n) before sending the response, to avoid confusing the HTTP framing. 2. Use Content-Length framing instead of chunked encoding, and send 4KB of body data (NSUrlSession may buffer very small payloads before calling DidReceiveData). 3. Declare a Content-Length 10x larger than the data actually sent, so the connection stays open and ReadAsync blocks in the polling loop waiting for more data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
✅ [PR Build #913e4f8] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
Collaborator
✅ [PR Build #913e4f8] Build passed (Build packages) ✅Pipeline on Agent |
Collaborator
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
Collaborator
✅ [PR Build #913e4f8] Build passed (Build macOS tests) ✅Pipeline on Agent |
Collaborator
🔥 [CI Build #913e4f8] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 91 tests failed, 102 tests passed. Failures❌ monotouch tests (iOS)20 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)23 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)23 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)20 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Ventura (13) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS testsLinux Build VerificationPipeline on Agent |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a test that sets up a stalling HTTP server, reads the first byte
successfully, then cancels the second ReadAsync via a caller-owned
CancellationToken.
TODO:
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
🤖 Pull request created by Copilot
Fixes: #25667