search

jaraco / irc

Full-featured Python IRC library for Python.
MIT License
390 stars 84 forks source link

Fix default behavior of hanging without entering reconnection strategy #196

Closed fitnub-bosbud closed 1 year ago

fitnub-bosbud commented 2 years ago

This change throws an exception on the first connection attempt that way the user can check server info and there is some indication of failure.

jaraco commented 2 years ago

I think maybe I understand the motivation for this change, but I'm not sure. Can you provide more detail about the situation that motivated you to make this change? Also, I'd like to come up with a solution that doesn't involve a flag, but instead has a more immediate way of detecting "first connection failure". Maybe by raising a different exception on the first connection attempt or by having a routine that encapsulates the initial connection state.

fitnub-bosbud commented 2 years ago

Sure thing, so basically the bot sits in an infinite loop if it initially fails to connect to a server. Here's a video example. I'd be open to coming up with another solution, but I think a flag is the easiest approach since the socket.error is raised from multiple inheritance, and this issue seems bot specific.

https://user-images.githubusercontent.com/82548166/137050294-d7a52abf-0ce3-474d-8f53-2d5661b90454.mp4

fitnub-bosbud commented 2 years ago

bump

jaraco commented 2 years ago

The more I look at this, the less sure I am that it's the right behavior. Why should the behavior be to ignore connection failures but only on subsequent attempts? Unfortunately, the behavior isn't well-documented nor tested, so I have low confidence in the intent of the existing code. As you mention in the title, maybe the logic should be re-worked so that the reconnection strategy is employed when the initial connection fails. I'd like to see a more deliberate design rather than a tweak to the code.

fitnub-bosbud commented 2 years ago

You make a good point. Instead of a hacked together workaround I utilize the existing functionality of the client to trigger a disconnect event. If I had to guess this was probably close to what the initial author intended and perhaps forgot to implement with the 

except:
    pass
fitnub-bosbud commented 1 year ago

bump

fitnub-bosbud commented 1 year ago

this is the change you requested

jaraco commented 1 year ago

Thanks for the ping. Released as v20.1.0. Let's see how it goes.