NSUrlSessionHandler should propagate network exceptions #18459
Conversation
|
@microsoft-github-policy-service agree company="SnagR Ltd." |
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 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.
| // set the stream as finished | ||
| inflight.Stream.TrySetReceivedAllData (); | ||
|
|
There was a problem hiding this comment.
For others, since I had to look into why this a little deeper and is missing some context. This change has no 'difference' in the final result. The reason for it is that TrySetException in the NSUrlSessionDataTaskStream. That method does the following:
public void TrySetException (Exception e)
{
exc = e;
TrySetReceivedAllData ();
}Therefore, in case of an exception, the stream ends up in the same situation. Moving it to the else is probably fixing a run condition in which we set it as completed but the exception is not set in the stream.
| @@ -1334,8 +1334,10 @@ public override async Task<int> ReadAsync (byte [] buffer, int offset, int count | |||
|
|
|||
| while (current is null) { | |||
| lock (dataLock) { | |||
| if (data.Count == 0 && receivedAllData && position == length) | |||
| if (data.Count == 0 && receivedAllData && position == length) { | |||
| ThrowIfNeeded (cancellationToken); | |||
There was a problem hiding this comment.
Needed, else won't throw +1
|
@rolfbjarne @dalexsoto I believe the is a valid change and the failures in the CI are not related to this commit, but due to an update on the allowed words by Microsoft in the source code, specially in the mon source code: And some nonsense related to the Mac APIs: |
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻✅ All tests on macOS M1 - Mac Ventura (13.0) passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11.5) passed. Pipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ Generator diffGenerator diff is empty Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: simulator tests. 🎉 All 235 tests passed 🎉 Tests counts✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
fixes #18458