Skip to content
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

Fire the pipelines error caught method when NWConnection's state changes to failed #187

Merged
merged 7 commits into from
Oct 25, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ extension StateManagedNWConnectionChannel {
self.connectPromise = nil
pendingConnect.fail(error)
}

// Step 4 Forward the connection state failed Error
let channelError = error as? ChannelError
if channelError != .eof {
self.pipeline.fireErrorCaught(error)
}
}

public func doHalfClose0(error: Error, promise: EventLoopPromise<Void>?) {
Expand Down
56 changes: 56 additions & 0 deletions Tests/NIOTransportServicesTests/NIOTSConnectionChannelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -865,5 +865,61 @@ class NIOTSConnectionChannelTests: XCTestCase {
.wait()
XCTAssertNoThrow(try connection.close().wait())
}

func testErrorIsForwardedFromFailedConnectionState() throws {
final class ForwardErrorHandler: ChannelDuplexHandler {
typealias OutboundIn = ByteBuffer
typealias InboundIn = ByteBuffer

private let testCompletePromise: EventLoopPromise<Error>
let listenerChannel: Channel

init(testCompletePromise: EventLoopPromise<Error>, listenerChannel: Channel) {
self.testCompletePromise = testCompletePromise
self.listenerChannel = listenerChannel
}
func channelActive(context: ChannelHandlerContext) {
let listenerErrorPromise = self.listenerChannel.eventLoop.next().makePromise(of: Error.self)
listenerChannel
.close()
.whenSuccess { _ in
context.channel.write(ByteBuffer(data: Data()))
.whenFailure({ error in
listenerErrorPromise.succeed(error)
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this path should complete the promise: we're testing for this happening in errorcaught, not on the write promise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you cool with me just throwing the result of the write away?

}
}

func errorCaught(context: ChannelHandlerContext, error: Error) {
let error = error as? ChannelError
XCTAssertNotEqual(error, ChannelError.eof)
XCTAssertEqual(error, ChannelError.ioOnClosedChannel)
Lukasa marked this conversation as resolved.
Show resolved Hide resolved
XCTAssertNotNil(error)
testCompletePromise.succeed(error!)
}
}

let listener = try NIOTSListenerBootstrap(group: self.group)
.childChannelInitializer { channel in
return channel.eventLoop.makeSucceededVoidFuture()
}
.bind(host: "127.0.0.1", port: 8080)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we bind 0 instead? This can cause awkward conflicts if this port is in use somewhere else.

.wait()

let testCompletePromise = self.group.next().makePromise(of: Error.self)
let connection = try NIOTSConnectionBootstrap(group: self.group)
.channelInitializer { channel in
channel.pipeline.addHandler(
ForwardErrorHandler(
testCompletePromise: testCompletePromise,
listenerChannel: listener
)
)
}
.connect(host: "127.0.0.1", port: 8080)
.wait()
XCTAssertNoThrow(try connection.close().wait())
XCTAssertNoThrow(try testCompletePromise.futureResult.wait())
}
}
#endif