search

jaraco / irc

Full-featured Python IRC library for Python.
MIT License
392 stars 87 forks source link

Fixes #119 per comments #120

Closed strictlymike closed 7 years ago

strictlymike commented 7 years ago

Hi @jaraco, I wrote this to handle cases such as where an IRC client might initiate a PING command and then immediately disconnect. Without this fix, the serve_forever() routine emits exception output on the console whenever this occurs. In my testing, this fix causes it to disconnect gracefully. Let me know if I'm missing anything. Thanks!

jaraco commented 7 years ago

This looks reasonable. I'm not familiar with the code so I have one question I'd like to answer first. Are there socket errors that shouldn't trigger a disconnect? If no, then this change is just fine.

Can you also add a changelog entry to CHANGES.rst? It should be under a new section for a 0.0.1 bump.

strictlymike commented 7 years ago

It's not clear to me from the documentation, but in the interest of handling the most narrow case possible to avoid masking errors, I've added a check for the specific case of the broken pipe. And I've updated CHANGES.rst. I tested with the following:

srv = IRCServer(('127.0.0.1', 6667), IRCClient) srv.serve_forever()

And it worked with my misbehaving client that calls srv.ping() and immediately hangs up.

Let me know if you see anything else needing attention. Thanks!

jaraco commented 7 years ago

As the continuous integration tests have caught, this introduces a SyntaxError on Python 3 - just needs except as.

strictlymike commented 7 years ago

Thanks, sorry about that! Updated, retested, and retesting locally with python3 for good measure before I push again.

jaraco commented 7 years ago

Looks great. And I can cut a release with a simple tag.

strictlymike commented 7 years ago

Rock_on. Thanks!