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.

Sign up for GitHub

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

Add requirement that channel is accessible in ServerChildChannel.Value #450

Merged
merged 4 commits into from
May 15, 2024
Merged

Add requirement that channel is accessible in ServerChildChannel.Value #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f3c958
Add requirement that channel is accessible in ServerChildChannel.Value
adam-fowler May 14, 2024
5e46f04
Update Sources/HummingbirdHTTP2/HTTP2Channel.swift
adam-fowler May 14, 2024
c235687
Update Sources/HummingbirdHTTP2/HTTP2Channel.swift
adam-fowler May 14, 2024
3dd229c
Update Sources/HummingbirdHTTP2/HTTP2Channel.swift
adam-fowler May 14, 2024
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
7 changes: 7 additions & 0 deletions Sources/HummingbirdCore/Server/HTTP/HTTP1Channel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,10 @@ public struct HTTP1Channel: ServerChildChannel, HTTPChannelHandler {
public let responder: @Sendable (Request, Channel) async throws -> Response
let additionalChannelHandlers: @Sendable () -> [any RemovableChannelHandler]
}

/// Extend NIOAsyncChannel to ServerChildChannelValue so it can be used in a ServerChildChannel
#if hasFeature(RetroactiveAttribute)
extension NIOAsyncChannel: @retroactive ServerChildChannelValue {}
#else
extension NIOAsyncChannel: ServerChildChannelValue {}
#endif
10 changes: 8 additions & 2 deletions Sources/HummingbirdCore/Server/ServerChildChannel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,15 @@ import Logging
import NIOCore
import ServiceLifecycle

/// HTTPServer child channel setup protocol
/// Protocol for typed server child channel
public protocol ServerChildChannelValue: Sendable {
/// Child channel that spawned child channel
var channel: Channel { get }
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it better if we add var eventLoop: EventLoop here? This assumes we're using a NIO.Channel, whereas we could have other implementations such as Lambda that aren't connected to a socket at all.

I would even consider removing this requirement entirely, and replacing it with something like this:

func addTask<Failure: Error>(
  _ operation: @escaping () async throws -> Void,
  to taskGroup: inout ThrowingDiscardingTaskGroup<Failure>
)

Copy link
Member Author

Choose a reason for hiding this comment

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

The ServerChildChannel assumes we are using a NIO.Channel anyway. It is there to setup a NIO child channel. This code is never run on lambda.

I'm not exactly sure how you would use the addTask function

}

/// Generic server child channel setup protocol
public protocol ServerChildChannel: Sendable {
associatedtype Value: Sendable
associatedtype Value: ServerChildChannelValue

/// Setup child channel
/// - Parameters:
Expand Down
9 changes: 7 additions & 2 deletions Sources/HummingbirdHTTP2/HTTP2Channel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import NIOSSL

/// Child channel for processing HTTP1 with the option of upgrading to HTTP2
public struct HTTP2UpgradeChannel: HTTPChannelHandler {
public typealias Value = EventLoopFuture<NIONegotiatedHTTPVersion<HTTP1Channel.Value, (NIOAsyncChannel<HTTP2Frame, HTTP2Frame>, NIOHTTP2Handler.AsyncStreamMultiplexer<HTTP1Channel.Value>)>>
public struct Value: ServerChildChannelValue {
let negotiatedHTTPVersion: EventLoopFuture<NIONegotiatedHTTPVersion<HTTP1Channel.Value, (NIOAsyncChannel<HTTP2Frame, HTTP2Frame>, NIOHTTP2Handler.AsyncStreamMultiplexer<HTTP1Channel.Value>)>>
public let channel: Channel
}

private let sslContext: NIOSSLContext
private let http1: HTTP1Channel
Expand Down Expand Up @@ -91,6 +94,8 @@ public struct HTTP2UpgradeChannel: HTTPChannelHandler {
}.flatMapThrowing {
try HTTP1Channel.Value(wrappingChannelSynchronously: http2ChildChannel)
}
}.map {
.init(negotiatedHTTPVersion: $0, channel: channel)
}
}

Expand All @@ -100,7 +105,7 @@ public struct HTTP2UpgradeChannel: HTTPChannelHandler {
/// - logger: Logger to use while processing messages
public func handle(value: Value, logger: Logger) async {
do {
let channel = try await value.get()
let channel = try await value.negotiatedHTTPVersion.get()
switch channel {
case .http1_1(let http1):
await handleHTTP(asyncChannel: http1, logger: logger)
Expand Down
Loading