search

python-websockets / websockets

Library for building WebSocket servers and clients in Python
https://websockets.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
5.23k stars 519 forks source link

ServerConnection._aiter_ not raising websockets.execptions.ConnectionClosed #1532

Closed Electron4444 closed 3 weeks ago

Electron4444 commented 3 weeks ago

I purpose a slight change to the ServerConnection.aiter method, to raise the ConnectionClosed Exception on excepting the CloseConnectionOK Exception. This would lead to the funtionality documented in Patterns for the Producer to be true for the Consumer as well and enable the possibility for a server waiting on receiving messages to close the handler for the corresponding client more easily on a connection close

aaugustin commented 3 weeks ago

I don't want to do this for two reasons:

  1. I think that the current API is the right one. Iterating until the end of a connection closed normally isn't a problem, therefore it shouldn't raise an exception.
  2. It's a significant backwards-incompatibility: it will trigger an exception in an extremely common code path. Even if it was an improvement, probably I wouldn't make the change for that reason.
Electron4444 commented 3 weeks ago

Could you maybe add a keyword option to set which enables this. I'm gonna make a pull- request containing the proposal. I would just let this controlling optional parameter default to None but have it raise the exception if enabled. This would make handeling Connection Closes inside the handler function provided to serve way easier.

aaugustin commented 3 weeks ago

No. Good API design is about providing a good API, not about providing knobs to get every possible behavior.

The problem with the change that you're proposing is that a reader of the code cannot tell anymore what for message in websocket: ... does. The reader needs to look at where the websocket object is defined. That's bad.