python-engineio erroneously ignores ValueError exceptions from applications

[lericson]

People generally want to know when their code crashes. However, https://github.com/miguelgrinberg/python-engineio/blob/master/engineio/asyncio_socket.py#L182 explicitly ignores ValueErrors. There is no rationale, there is no explanation. Please remove this ASAP.

[miguelgrinberg]

People generally want to know when their code crashes

I think you need to read up on Exceptions more. Crashes raise exceptions, that's true, but exceptions are also used as a signaling mechanism, which is the case in this snippet of code you are indicated, which receives a signal from https://github.com/miguelgrinberg/python-engineio/blob/master/engineio/asyncio_socket.py#L47.

[lericson]

Oh. I see. Interesting. Insulting your users by insinuating they don't know how Python. I know what exceptions are. I've used Python for a decade. This one is called Value Error, and as the name suggests, is meant to signal one thing: errors.

Co-opting Python's most common exception type to mean "unhandled WebSocket packet type" seems ill-conceived, especially considering the user very likely wants to know when packets are being dropped. All in all, a pretty bad way to deal with the problem.

More so, if your user's code errors with that exception type, which is raised by basically every single library both standard and PyPI, your library simply silences it -- because it might mean the last WebSocket packet's type isn't recognized by the library.

I urge you to rethink this design.

[miguelgrinberg]

I did not mean to insult you. You suggested I was suppressing ValueError crashes from the application, I'm telling you this specific instance of ValueError is not an application crash.

Have you found a situation in which a ValueError raised in any other part of the code, including your own application is caught in the place you indicated? If you did, then please report the specific error you have found and I'll consider it a bug to be fixed.

[lericson]

Sure, just raise a ValueError from a @sio.on("foo") handler with python-socketio.

[lericson]

I'm using aiohttp, I can construct a minimal case if you would like me to. I doubt it's necessary though.

[lericson]

Again though, I'm not familiar enough with the internals of the code to know, but silencing unrecognized packet types seems like trouble down the line. I think emitting a warning log message for unrecognized packet types seems more fitting.

[miguelgrinberg]

Oh, now I see what you mean, I was missing the context to understand what you meant.

[lericson]

I'm sorry, I was perhaps not very clear. 😄 Happy we're on the same page.