Websocket Error Propagation & Handling

[thebarndog]

I've been using the Websocket api recently and one thing that I've been struggling with is how to best handle errors and propagate them to the client, similar to what's discussed in #2434. The problem is particular pervasive when using Swift concurrency because the the apis are all marked as async throws which leads to situations like this:

app.webSocket("conversation") { request, socket -> Void in
    try await socket.send("Welcome!")
}

but the onUpgrade closure parameter is only async and these errors then have to be handled somehow which gets messy, fast.

To unblock myself so I can at least write simple straight line code without having to worry about excessive error handling for the moment, I made some extensions that model the pre-existing websocket extensions:

extension Vapor.Application {
    
    @preconcurrency
    @discardableResult
    func webSocket(
        _ path: PathComponent...,
        maxFrameSize: WebSocketMaxFrameSize = .`default`,
        shouldUpgrade: @escaping (@Sendable (Request) async throws -> HTTPHeaders?) = { _ in [:] },
        onUpgrade: @Sendable @escaping (Request, WebSocket) async throws -> ()
    ) -> Route {
        self.webSocket(path, maxFrameSize: maxFrameSize, shouldUpgrade: shouldUpgrade, onUpgrade: onUpgrade)
    }
}

extension RoutesBuilder {
    
    @preconcurrency
    @discardableResult
    func webSocket(
        _ path: PathComponent...,
        maxFrameSize: WebSocketMaxFrameSize = .`default`,
        shouldUpgrade: @escaping (@Sendable (Request) async throws -> HTTPHeaders?) = { _ in [:] },
        onUpgrade: @Sendable @escaping (Request, WebSocket) async throws -> ()
    ) -> Route {
        self.webSocket(path, maxFrameSize: maxFrameSize, shouldUpgrade: shouldUpgrade, onUpgrade: onUpgrade)
    }
    
    @preconcurrency
    @discardableResult
    func webSocket(
        _ path: [PathComponent],
        maxFrameSize: WebSocketMaxFrameSize = .`default`,
        shouldUpgrade: @escaping (@Sendable (Request) async throws -> HTTPHeaders?) = { _ in [:] },
        onUpgrade: @Sendable @escaping (Request, WebSocket) async throws -> ()
    ) -> Route {
        return self.on(.GET, path) { request -> Response in
            return request.webSocket(maxFrameSize: maxFrameSize, shouldUpgrade: shouldUpgrade, onUpgrade: onUpgrade)
        }
    }
}

extension Request {

    /// Upgrades an existing request to a websocket connection
    @preconcurrency
    public func webSocket(
        maxFrameSize: WebSocketMaxFrameSize = .`default`,
        shouldUpgrade: @escaping (@Sendable (Request) async throws -> HTTPHeaders?) = { _ in [:] },
        onUpgrade: @Sendable @escaping (Request, WebSocket) async throws -> ()
    ) -> Response {
        webSocket(
            maxFrameSize: maxFrameSize,
            shouldUpgrade: { request in
                let promise = request.eventLoop.makePromise(of: HTTPHeaders?.self)
                promise.completeWithTask {
                    try await shouldUpgrade(request)
                }
                return promise.futureResult
            },
            onUpgrade: { request, socket in
                Task {
                    try await onUpgrade(request, socket)
                }
            }
        )
    }
}

but this obviously is not great because the problem hasn't been solved, just shifted and hidden. The error is still being swallowed, albeit in a different place. I can write my code cleanly right now but if I can't control how and where errors are propagated, then that's not great.

One thought I had was introducing an onError hander that could be called when something in onUpgrade fails. That way at least the code inside onUpgrade is much cleaner and the error handling that would have been done there can now be done in the handler.

@tanner0101 any thoughts on this? I know you were mulling this over in #2434, maybe your thinking has evolved on how to handle this in the past few years. It might be worth to start formalizing ideas around given that Vapor is the de-facto Swift server framework given that it sees active development and Kitura and Perfect haven't released in some time.