WebSocket Router #41
WebSocket Router #41
Conversation
| } | ||
|
|
||
| func testRouteSelectionFail() async throws { | ||
| try XCTSkipIf(true) |
There was a problem hiding this comment.
Why are we skipping this test? Shouldn't we still verify that websockets are not always upgraded?
There was a problem hiding this comment.
This is a temporary thing should have mentioned this. The tests hangs and I havent worked out why this is happening
| let logger = { | ||
| var logger = Logger(label: "WebSocketTest") | ||
| logger.logLevel = .debug | ||
| return logger | ||
| }() |
There was a problem hiding this comment.
| let logger = { | |
| var logger = Logger(label: "WebSocketTest") | |
| logger.logLevel = .debug | |
| return logger | |
| }() | |
| var logger = Logger(label: "WebSocketTest") | |
| logger.logLevel = .debug |
There was a problem hiding this comment.
That's means we have a mutable logger, which we cannot use in a child task
| /// - parameters | ||
| public static func webSocketUpgrade<ResponderBuilder: HTTPResponderBuilder>( | ||
| additionalChannelHandlers: @autoclosure @escaping @Sendable () -> [any RemovableChannelHandler] = [], | ||
| maxFrameSize: Int = 1 << 14, |
There was a problem hiding this comment.
Can we make this a parameter configuration: WebSocketServerConfiguration?
maxFrameSize is definitely the most important configurable property, but I can imagine us adding a few more in the future.
| public init<Context: WebSocketRequestContext, ResponderBuilder: HTTPResponderBuilder>( | ||
| additionalChannelHandlers: @escaping @Sendable () -> [any RemovableChannelHandler] = { [] }, | ||
| responder: @escaping @Sendable (Request, Channel) async throws -> Response = { _, _ in throw HTTPError(.notImplemented) }, | ||
| maxFrameSize: Int = (1 << 14), |
There was a problem hiding this comment.
Can we make this a parameter configuration: WebSocketServerConfiguration?
| @@ -19,16 +19,16 @@ import NIOCore | |||
| public protocol WebSocketContextProtocol: Sendable { | |||
| var logger: Logger { get } | |||
| var allocator: ByteBufferAllocator { get } | |||
| init(logger: Logger, allocator: ByteBufferAllocator) | |||
| init(channel: Channel, logger: Logger) | |||
| } | |||
|
|
|||
| /// Default implementation of ``WebSocketContextProtocol`` | |||
| public struct WebSocketContext: WebSocketContextProtocol { | |||
| public let logger: Logger | |||
| public let allocator: ByteBufferAllocator | |||
There was a problem hiding this comment.
I can imagine that the remote address is useful
There was a problem hiding this comment.
Likewise the original HTTPRequest could be useful in a WebSocketContextProtocol?
There was a problem hiding this comment.
This is basically a duplicate of the requirements from the RequestContext so including the request might be hard. You get the request in the shouldUpgrade call so things like permessage-deflate can be negotiated there
|
Fixed issues with WebSocket upgrade fail not sending response. Solution isn't great, hopefully will get better solution from NIO at some point. |
| package struct RequestID: CustomStringConvertible { | ||
| let low: UInt64 | ||
|
|
||
| package init() { | ||
| self.low = Self.globalRequestID.loadThenWrappingIncrement(by: 1, ordering: .relaxed) | ||
| } | ||
|
|
||
| package var description: String { | ||
| Self.high + self.formatAsHexWithLeadingZeros(self.low) | ||
| } | ||
|
|
||
| func formatAsHexWithLeadingZeros(_ value: UInt64) -> String { | ||
| let string = String(value, radix: 16) | ||
| if string.count < 16 { | ||
| return String(repeating: "0", count: 16 - string.count) + string | ||
| } else { | ||
| return string | ||
| } | ||
| } | ||
|
|
||
| private static let high = String(UInt64.random(in: .min ... .max), radix: 16) | ||
| private static let globalRequestID = ManagedAtomic<UInt64>(UInt64.random(in: .min ... .max)) | ||
| } |
Initialise websocket upgrade with a router. This can be the same router as the one you use for everything else or a separate router, which would be more optimal given you are testing less routes for the upgrade.
This PR also removes any NIOHTTP1 types from the public interface