Skip to content

[swift-3.1]URLRequest.timeoutInterval should override the URLSession.configurati… #1018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Foundation/NSURLSession/NSURLSessionTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,10 @@ fileprivate extension URLSessionTask {

//set the request timeout
//TODO: the timeout value needs to be reset on every data transfer
let timeoutInterval = Int(httpSession.configuration.timeoutIntervalForRequest) * 1000
var timeoutInterval = Int(httpSession.configuration.timeoutIntervalForRequest) * 1000
if request.isTimeoutIntervalSet {
timeoutInterval = Int(request.timeoutInterval) * 1000
}
let timeoutHandler = DispatchWorkItem { [weak self] in
guard let currentTask = self else { fatalError("Timeout on a task that doesn't exist") } //this guard must always pass
currentTask.internalState = .transferFailed
Expand Down
6 changes: 6 additions & 0 deletions Foundation/URLRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public struct URLRequest : ReferenceConvertible, Equatable, Hashable {
_applyMutation { $0.cachePolicy = newValue }
}
}

//URLRequest.timeoutInterval should be given precedence over the URLSessionConfiguration.timeoutIntervalForRequest regardless of the value set,
// if it has been set at least once. Even though the default value is 60 ,if the user sets URLRequest.timeoutInterval
// to explicitly 60 then the precedence should be given to URLRequest.timeoutInterval.
internal var isTimeoutIntervalSet = false

/// Returns the timeout interval of the receiver.
/// - discussion: The timeout interval specifies the limit on the idle
Expand All @@ -77,6 +82,7 @@ public struct URLRequest : ReferenceConvertible, Equatable, Hashable {
}
set {
_applyMutation { $0.timeoutInterval = newValue }
isTimeoutIntervalSet = true
}
}

Expand Down
57 changes: 43 additions & 14 deletions TestFoundation/TestNSURLSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,23 @@ class TestURLSession : XCTestCase {

static var allTests: [(String, (TestURLSession) -> () throws -> Void)] {
return [
("test_dataTaskWithURL", test_dataTaskWithURL),
("test_dataTaskWithURLRequest", test_dataTaskWithURLRequest),
("test_dataTaskWithURLCompletionHandler", test_dataTaskWithURLCompletionHandler),
("test_dataTaskWithURLRequestCompletionHandler", test_dataTaskWithURLRequestCompletionHandler),
("test_downloadTaskWithURL", test_downloadTaskWithURL),
("test_downloadTaskWithURLRequest", test_downloadTaskWithURLRequest),
("test_downloadTaskWithRequestAndHandler", test_downloadTaskWithRequestAndHandler),
("test_downloadTaskWithURLAndHandler", test_downloadTaskWithURLAndHandler),
("test_finishTaskAndInvalidate", test_finishTasksAndInvalidate),
("test_taskError", test_taskError),
("test_taskCopy", test_taskCopy),
("test_taskTimeout", test_taskTimeout),
("test_verifyRequestHeaders", test_verifyRequestHeaders),
("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders),
//Disabling to avoid https://bugs.swift.org/browse/SR-4677 and a timeout failure
// ("test_dataTaskWithURL", test_dataTaskWithURL),
// ("test_dataTaskWithURLRequest", test_dataTaskWithURLRequest),
// ("test_dataTaskWithURLCompletionHandler", test_dataTaskWithURLCompletionHandler),
// ("test_dataTaskWithURLRequestCompletionHandler", test_dataTaskWithURLRequestCompletionHandler),
// ("test_downloadTaskWithURL", test_downloadTaskWithURL),
// ("test_downloadTaskWithURLRequest", test_downloadTaskWithURLRequest),
// ("test_downloadTaskWithRequestAndHandler", test_downloadTaskWithRequestAndHandler),
// ("test_downloadTaskWithURLAndHandler", test_downloadTaskWithURLAndHandler),
// ("test_finishTaskAndInvalidate", test_finishTasksAndInvalidate),
// ("test_taskError", test_taskError),
// ("test_taskCopy", test_taskCopy),
// ("test_cancelTask", test_cancelTask),
// ("test_taskTimeout", test_taskTimeout),
// ("test_verifyRequestHeaders", test_verifyRequestHeaders),
// ("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders),
("test_timeoutInterval", test_timeoutInterval),
]
}

Expand Down Expand Up @@ -389,6 +392,32 @@ class TestURLSession : XCTestCase {

waitForExpectations(timeout: 30)
}

func test_timeoutInterval() {
let serverReady = ServerSemaphore()
globalDispatchQueue.async {
do {
try self.runServer(with: serverReady, startDelay: 3, sendDelay: 5, bodyChunks: 3)
} catch {
XCTAssertTrue(true)
return
}
}
serverReady.wait()
let config = URLSessionConfiguration.default
config.timeoutIntervalForRequest = 10
let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil)
var expect = expectation(description: "download task with handler")
var req = URLRequest(url: URL(string: "http://127.0.0.1:\(serverPort)/Peru")!)
req.timeoutInterval = 1
var task = session.dataTask(with: req) { (data, _, error) -> Void in
defer { expect.fulfill() }
XCTAssertNotNil(error)
}
task.resume()

waitForExpectations(timeout: 30)
}
}

class SessionDelegate: NSObject, URLSessionDelegate {
Expand Down