Websocket Error Propagation & Handling
thebarndog opened this issue · 0 comments
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.