sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
951 stars 403 forks source link

Issues with .whois #55

Closed Relboar closed 12 years ago

Relboar commented 12 years ago

There is a few problems with the .whois command:

Example:

.whois ChanServ AttributeError: 'NoneType' object has no attribute 'split' (file "/home/bnc/jenni/modules/whois.py", line 23, in sendwhois) .whois User AttributeError: 'NoneType' object has no attribute 'split' (file "/root/bnc/jenni/modules/whois.py", line 23, in sendwhois) .whois Bot [WHOIS] Nick: ChanServ Host: service@rizon.net Real name: Channel ChanServ is on 1 channels: @#bot-debug +#bot
embolalia commented 12 years ago

The channel count is already fixed (I think it has been for some time, but I may be mistaken). I'm unable to confirm that last issue, but I think it should resolve itself if the error is fixed. I'll try to get to this today.

embolalia commented 12 years ago

The realname issue is now fixed.

The number of channels issue is strange. It doesn't happen for /all/ users on only one channel. I've also started seeing it happen with @tyrope, who's on multiple channels. (And it does seem to be the same problem; it throws the exception at the same point.)

In the mean time, it's a known issue that when .whois fails, it tends to screw up all the future whoises until you reload the module. So until we can figure out why it's failing, all I can suggest is reloading it if you see it fail.

lramati commented 12 years ago

could we write exception catching code reload modules that fail in general and just say that it was done rather than leave people to figure out it crashed? regular users could easily botch a command (especially like whois) but not be capable of using reload, which might lead to modules that are broken but not known to be so. This would help the bot stop from breaking down when there are no mods around to fix it.

elad661 commented 12 years ago

So, let's take a close analysis of why it failed and how I fixed it.

This module relies on quite a lot of state and information variables that are shared between its functions. This is fine, probably could be written a bit better (putting them in a class and having a .clear() function to clear them comes to mind).

So, the actual reason for the breakage was that when you got an invalid user, or a user that is in 0 channels, an exception will be raised, and the actual code in the end of the file that rests those vars will never be executed, thus keeping the vars as they are.

The next time someone will attempt to use whois, it will check if it got 318 (IRC code for whois response), and according to the state var (which was not reset), it did. So it happily goes to parse the data, but the data isn't actually the new data it just got from the server, but the data which caused whois to raise an exception, thus making the module non-functional until it is reloaded (variable values clear on reload).

Gracefully handling these two cases makes sure the vars will always be reset. I might add a try and finally blocks to make sure that even if there's an exception we did not predict, the vars will always be reset.

Closing.

elad661 commented 12 years ago

Note that whois is not yet perfect: if the server is a bit slow or if two users ask for whois at the same time, response might not behave as expected and output wrong data. I will fix that later.

lramati commented 12 years ago

i am working for an alternate fix for this.