Example client exits on ConnectionClosed (#22) #55
Conversation
This is the second attempt at this. Instead of modifying trio-websocket to support cancelling a handler, this commit modifies only the example client itself: it now has one task that gets messages and prints them, and a second task that gets user input and sends messages/commands/etc.
|
I'm not a fan of splitting send and receive to resolve this issue, as it's creating another state machine. We should use concurrency to avoid coding explicit state machines (http://250bpm.com/blog:69). In a real program doing query-with-reply over websocket, it's much simpler to keep the send and receive within a single imperative control flow. If the heartbeat pattern from the other CL works I'd rather this example use it, and keep the send / receive together. |
|
Copying @belm0 from the "ping v2" thread:
Removing the reader task can't be done without changing the library's implementation, though, i.e. adding one of the "connection is closed" signals we talked about. Or are you suggesting the heartbeat is used for detecting a closed connection? If so, that amounts to polling the connection state (and it generates unnecessary packets if all we care about is detecting that the local socket closed), which is a pattern I don't want to encourage. I did read the blog post you sent about state machines, but I must confess that I can't grasp its implications for this PR. The example client feels pretty simple to me. I don't see any drawback to having a separate reader task. Indeed, I think many applications that multiplex requests and responses on a single WebSocket will need to have a separate reader task, so it's not a pattern we want to avoid. |
|
@belm0 Unless you strongly object, I am going to merge this as-is. I don't see where the added state machine is, and I think that separate readers and writers will be a common set up for users. |
|
Adding state would likely be the next logical step: the reader task would need to know what to expect on the line, e.g. if it becomes responsible for dealing with the application-level protocol. Being an example, I think it's important to minimize the number of tasks and keep things as local as possible, and hence easy to follow. There is a class of websocket use where one side drives the messaging, and this message echo toy falls under that. So it would be great to keep the send and receive together. Keeping that simplicity seems more important than the ideal of having the client automatically shut down on a closed connection, so I'd opt to leave things as is given a choice between the two. I'll defer to you-- thanks for considering. |
|
Of course it's just an example, so it doesn't really have to meet any particular spec. But I was imagining it as a kind of websocket test client: if it lets you connect, send arbitrary messages, and has a background loop running that prints any messages it receives, then that's sufficient to manually implement any websocket protocol, including ones that don't follow a strict request/response pattern. (And presumably most websocket protocols don't follow strict request/response patterns, since that's what HTTP does!) |
This is the second attempt at this. Instead of modifying
trio-websocket to support cancelling a handler, this commit
modifies only the example client itself: it now has one task
that gets messages and prints them, and a second task that
gets user input and sends messages/commands/etc.
This supersedes #48.