Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 Sources/AsyncHTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,10 @@ public class HTTPClient {
}
}.always { _ in
setupComplete.succeed(())
}.cascadeFailure(to: task.promise)
}.whenFailure { error in
delegate.didReceiveError(task: task, error)
task.promise.fail(error)
}

return task
}
Expand Down
1 change: 1 addition & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ extension HTTPClientTests {
("testNothingIsLoggedAtInfoOrHigher", testNothingIsLoggedAtInfoOrHigher),
("testAllMethodsLog", testAllMethodsLog),
("testClosingIdleConnectionsInPoolLogsInTheBackground", testClosingIdleConnectionsInPoolLogsInTheBackground),
("testConnectErrorPropagatedToDelegate", testConnectErrorPropagatedToDelegate),
]
}
}
34 changes: 34 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2005,4 +2005,38 @@ class HTTPClientTests: XCTestCase {

self.defaultClient = nil // so it doesn't get shut down again.
}

func testConnectErrorPropagatedToDelegate() throws {
class TestDelegate: HTTPClientResponseDelegate {
typealias Response = Void
var error: Error?
func didFinishRequest(task: HTTPClient.Task<Void>) throws {}
func didReceiveError(task: HTTPClient.Task<Response>, _ error: Error) {
self.error = error
}
}

let httpBin = HTTPBin()
let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
defer {
XCTAssertNoThrow(try httpClient.syncShutdown())
}

let delegate = TestDelegate()
let request = try HTTPClient.Request(url: "http://localhost:\(httpBin.port)/get")
do {
try httpBin.shutdown()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be in an XCTAssertNoThrow

_ = try httpClient.execute(request: request, delegate: delegate).wait()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use

XCTThrowsError(try httpClient.execute(...).wait()) { error in
   XCTAssert(...)
}

we literally had security issues before because of this pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, thank you for catching this!

XCTFail("Should fail")
} catch {
switch (error, delegate.error) {
case (_ as NIOConnectionError, _ as NIOConnectionError):
break
case (_ as NIOConnectionError, .none):
XCTFail("Delegate error is not \(error)")
default:
XCTFail("Unexpected error: \(error)")
}
}
}
}