vapor / http

🚀 Non-blocking, event-driven HTTP built on Swift NIO.
MIT License
238 stars 65 forks source link

Working with async functions #350

Open grahamburgsma opened 5 years ago

grahamburgsma commented 5 years ago

Since the rest of Vapor is very asynchronous with many tasks returning futures, it makes it difficult to work with web sockets. Maybe this just requires more examples in the documentation, though it would be nice if web sockets got some love like the rest of Vapor.

If you need to do a query in one of the following examples, there is no where to return the future. I've run in to leaking promises and other issues because of this.

func onUpgrade(ws: WebSocket, req: Request) throws
ws.onText { (ws, text) in
    // no where to return future
}

Could this get addressed in Vapor 4?

tanner0101 commented 5 years ago

This could be possible, if we can figure out a sensible way for the closure to handle a failed future. Something like logging the error then closing the WebSocket?

I'm not sure how useful that would be though since one would probably want better error handling logic for their client's sake. Which you can do currently with something like:

ws.onText { (ws, text) in
    someAsyncThing().do { res in
        ws.send("res is: \(res")
    }.catch { error in
        ws.send("oops: \(error)")
        ws.close()
    }
}
calebkleveter commented 5 years ago

I think a WebSocket.onError handler would be nice:

ws.onError( (ws, error) in
    logger.error(error.localizedDescription)
    ws.close()
}
grahamburgsma commented 5 years ago

I like @calebkleveter's idea, though don't know if that would be possible. Plus we already have WebSocket.onError and it's used for when the connection itself has an error.

But one place to handle errors from futures, or maybe an error middleware for web sockets would be nice.