Flesh out URLError information upon URLSessionTask cancellation

[glessard]

The URLError received upon URLSessionTask cancellation by a URLSessionTaskDelegate or completion handler has a blank userInfo dictionary. This is an inconsistency when compared with macOS's Foundation and it makes actual error handling harder than it could be.

This PR fills it out to match the fields filled out by macOS Foundation.

I previously reported this as https://bugs.swift.org/browse/SR-13017, and I believe this fixes that (if a fix is even desirable, that is.)

[glessard]

There are 6 other uses of the URLError constructor in the FoundationNetworking project, and their userInfo situation is quite inconsistent. Would it be desirable to change that?

[spevans]

The testcase is ok, I ran it under DarwinCompatibilityTests and it works correctly against Darwin.

However the test failed on both Linux and macOS. Note that currently the TestURLSession tests are disabled and need to be re-enabled in the Tests/Foundation/main.swift file. URLSession still has some other issues that makes the tests generally flaky.

Note in this case it looks like request?.url was nil so the failingURL and failureURLString entries werent added.

[glessard]

I thought it ran in the TestFoundation target of the Xcode project, but maybe it didn't. (I should have tried to crash the test to make sure!) Building on Linux failed yesterday, and didn't have enough time to fix that so I crossed my fingers and submitted it. I'll try again.

[glessard]

@spevans I haven't had time to solve my Linux build, but now I've gotten the test to properly run from Xcode.

[spevans]

I think we should fix the URLError.failingURL and .failureURLString properties that your original tests exposed. The following seems to fix it in my tests:

diff --git a/Sources/Foundation/NSError.swift b/Sources/Foundation/NSError.swift
index 0b55deea..ff71279e 100644
--- a/Sources/Foundation/NSError.swift
+++ b/Sources/Foundation/NSError.swift
@@ -919,12 +919,12 @@ extension URLError {
 
     /// The URL which caused a load to fail.
     public var failingURL: URL? {
-        return _nsUserInfo[NSURLErrorFailingURLErrorKey._bridgeToObjectiveC()] as? URL
+        return _nsUserInfo[NSURLErrorFailingURLErrorKey] as? URL
     }
 
     /// The string for the URL which caused a load to fail.
     public var failureURLString: String? {
-        return _nsUserInfo[NSURLErrorFailingURLStringErrorKey._bridgeToObjectiveC()] as? String
+        return _nsUserInfo[NSURLErrorFailingURLStringErrorKey] as? String
     }
 }

[glessard]

I wasn't sure what _bridgeToObjectiveC() was supposed to do, so I didn't go that route. Also: shouldn't that be in a different PR?

This being said, there are other things with URLError: none of the properties added in iOS 13 are on it yet. Obviously they're for either darwin-specific or not-yet-supported things, but shouldn't they be there, returning nil?

[spevans]

Yes I think a separate PR to fix up the properties in URLError would be a good start. I think the

private var _nsUserInfo: [AnyHashable : Any] {
        return _nsError.userInfo
    }

can be changed to a [String: Any] and then the properties fixed up.

It just requires adding some tests constructing URLError and then testing the properties and validating the tests in the DarwinCompatibilityTests project which will run the tests against the native Foundation. It would be good to add in the missing properties as well.

[spevans]

@swift-ci test

[spevans]

I just tested it locally on Linux with the TestURLSession tests enabled and it passed ok.

[spevans]

@swift-ci test

[glessard]

Thanks!