Skip to content
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
22 changes: 21 additions & 1 deletion Sources/NIO/BaseSocketChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,27 @@ class BaseSocketChannel<T: BaseSocket>: SelectableChannel, ChannelCore {
// the `reset` event.
final func reset() {
self.readEOF()
self.close0(error: ChannelError.eof, mode: .all, promise: nil)

if self.socket.isOpen {
assert(self.lifecycleManager.isRegistered)
let error: IOError
// if the socket is still registered (and therefore open), let's try to get the actual socket error from the socket
do {
let result: Int32 = try self.socket.getOption(level: SOL_SOCKET, name: SO_ERROR)
if result != 0 {
// we have a socket error, let's forward
// this path will be executed on Linux (EPOLLERR) & Darwin (ev.fflags != 0)
error = IOError(errnoCode: result, reason: "connection reset (error set)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than passing reason can you pass function? That'll give us better debug strings here.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Lukasa there's no function that errors here, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just make one up. ;) The key advantage is that we get the textual description of the underlying errno, rather than your hard-coded guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

that's not how it's intended. The reason is there so it gives you a better message. I acknowledge that we had a bug here where that was worse. In case where you don't want to provide a reason as it would be "connect failed, errno \(errno)", you can use function: #function as that's shorter.

} else {
// we don't have a socket error, this must be connection reset without an error then
// this path should only be executed on Linux (EPOLLHUP, no EPOLLERR)
error = IOError(errnoCode: ECONNRESET, reason: "connection reset (no error set)")
}
self.close0(error: error, mode: .all, promise: nil)
} catch {
self.close0(error: error, mode: .all, promise: nil)
}
}
assert(!self.lifecycleManager.isRegistered)
}

Expand Down
12 changes: 6 additions & 6 deletions Sources/NIO/IO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ public struct IOError: Swift.Error {
///
/// - parameters:
/// - errorCode: the `errno` that was set for the operation.
/// - function: the function / syscall that caused the error.
/// - reason: what failed
/// -returns: the constructed reason.
private func reasonForError(errnoCode: Int32, function: StaticString) -> String {
private func reasonForError(errnoCode: Int32, reason: String) -> String {
if let errorDescC = strerror(errnoCode) {
let errorDescLen = strlen(errorDescC)
return errorDescC.withMemoryRebound(to: UInt8.self, capacity: errorDescLen) { ptr in
let errorDescPtr = UnsafeBufferPointer<UInt8>(start: ptr, count: errorDescLen)
return "\(function) failed: \(String(decoding: errorDescPtr, as: UTF8.self)) (errno: \(errnoCode)) "
return "\(reason): \(String(decoding: errorDescPtr, as: UTF8.self)) (errno: \(errnoCode)) "
}
} else {
return "\(function) failed: Broken strerror, unknown error: \(errnoCode)"
return "\(reason): Broken strerror, unknown error: \(errnoCode)"
}
}

Expand All @@ -75,9 +75,9 @@ extension IOError: CustomStringConvertible {
public var localizedDescription: String {
switch self.reason {
case .reason(let reason):
return reason
return reasonForError(errnoCode: self.errnoCode, reason: reason)
case .function(let function):
return reasonForError(errnoCode: self.errnoCode, function: function)
return reasonForError(errnoCode: self.errnoCode, reason: "\(function) failed")
}
}
}
Expand Down
1 change: 1 addition & 0 deletions Tests/NIOTests/ChannelTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ extension ChannelTests {
("testCloseSocketWhenReadErrorWasReceivedAndMakeSureNoReadCompleteArrives", testCloseSocketWhenReadErrorWasReceivedAndMakeSureNoReadCompleteArrives),
("testSocketFailingAsyncCorrectlyTearsTheChannelDownAndDoesntCrash", testSocketFailingAsyncCorrectlyTearsTheChannelDownAndDoesntCrash),
("testSocketErroringSynchronouslyCorrectlyTearsTheChannelDown", testSocketErroringSynchronouslyCorrectlyTearsTheChannelDown),
("testConnectWithECONNREFUSEDGetsTheRightError", testConnectWithECONNREFUSEDGetsTheRightError),
]
}
}
Expand Down
26 changes: 26 additions & 0 deletions Tests/NIOTests/ChannelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2214,6 +2214,32 @@ public class ChannelTests: XCTestCase {
XCTAssertNoThrow(try sc.closeFuture.wait())
XCTAssertNoThrow(try sc.syncCloseAcceptingAlreadyClosed())
}

func testConnectWithECONNREFUSEDGetsTheRightError() throws {
let group = MultiThreadedEventLoopGroup(numThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}
let serverSock = try Socket(protocolFamily: PF_INET, type: Posix.SOCK_STREAM)
// we deliberately don't set SO_REUSEADDR
XCTAssertNoThrow(try serverSock.bind(to: SocketAddress(ipAddress: "127.0.0.1", port: 0)))
let serverSockAddress = try! serverSock.localAddress()
XCTAssertNoThrow(try serverSock.close())

// we're just looping here to get a pretty good chance we're hitting both the synchronous and the asynchronous
// connect path.
for _ in 0..<64 {
do {
_ = try ClientBootstrap(group: group).connect(to: serverSockAddress).wait()
XCTFail("just worked")
} catch let error as IOError where error.errnoCode == ECONNREFUSED {
// OK
} catch {
XCTFail("unexpected error: \(error)")
}
}
}

}

fileprivate class VerifyConnectionFailureHandler: ChannelInboundHandler {
Expand Down