twisted.words irc server does not work with gaim

[Glyph]

When I attempt to connect, regardless of whether I specify a password or not, gaim hangs forever and doesn't consider me logged in.

[Jean-Paul Calderone]

I checked with ethereal. The login seems to be succeeding but the Twisted Words server doesn't send very many codes in its greeting. I suppose gaim might be waiting for something which it doesn't send before considering login complete.

[David Sturgis]

Itamar and I discovered that including the MOTD lines:

("375", ":- %(serviceName)s Message of the Day - "), ("372", ":- The message of the day is: THERE IS NO MESSAGE OF THE DAY. - "), ("376", ":End of /MOTD command.")

as part of twisted.words.service.IRCUser._welcomeMessages, immediately after the RPL_MYINFO line, is consistent with FreeNode server behavior, and is more acceptable to GAIM.

[David Sturgis]

Actually, you don't even need the 372 (actual message of the day) line, just 375 (RPL_MOTDSTART) and then 376 (RPL_ENDOFMOTD). hooray.

[Jean-Paul Calderone]

Awesome, thanks for the investigation. Now it should be easy for someone to adjust IRCUser to send these codes and make gaim/pidgin work.

[David Sturgis]

(In [25256]) Branching to 'pidgin-2385'

[therve]

Here we go:

1.

-            "serviceName": self.hostname,
+            "serviceName": "test",

Why's this change?

2.

+        self.name = nickname;

Please remove the semi-colon.

3. One flake:

   twisted/words/test/test_irc_service.py:8: 'time' imported but unused
   

Thanks!

[David Sturgis]

Itamar and I addressed Therve's review comments, and also added an actual test for Logins. As part of this, we also fixed some bugs found by the test (hostname is always "undefined", fake creation time, etc.)

[Jean-Paul Calderone]

Looking good. A few simple things:

[Jean-Paul Calderone]

Let me try that again.

First, some simple things:

1. In service.py, the new serverInfo attribute of the factory needs to be documented. In particular, all the keys (and their associated values) which it will have need documentation.
2. In test_service.py, the _assertGreeting method's new docstring has some trailing whitespace in it; it also documents AssertionError as an exception which may be raised, but I think it's actually twisted.trial.unittest.FailTest, or maybe self.failureException.
3. Further down in that file, the new messageType parameter to _response is undocumented.
4. testNickServLogin's docstring also has trailing whitespace. As long as you're modifying the function, you should rename it to comply with the naming convention - test_nickServLogin.
5. In test_irc_service.py
   1. numerous lines end in trailing whitespace
   2. methods should be separated by two blank lines; many are, but setUp and the first test method following it are separated by only one blank line.
   3. The dates in the copyright statement should be made accurate - 2009, not 2001-2008.

Next. The behavior actually being implemented is very bizarre. Now the server sends the MOTD (except there's never an MOTD so it just sends two lines about how there's not an MOTD) whenever it receives NICK. As a separate bug, the server allows multiple NICKs to be issued. This might be a reasonable thing to allow, but it's not being handled properly now. Please file a new ticket for dealing with that misbehavior.

So, anyway. This isn't quite the behavior of other IRC servers (at least, not freenode). Freenode won't send anything until it gets both USER and NICK. Then it sends the whole "greeting" sequence. Then it uses NickServ to challenge you for a password. It would probably be best to emulate this more closely.

Finally, I don't really understand a lot of the changes to service.py. The name attribute, at least, seems necessary to make the test pass (although it wasn't before - I didn't bother to figure out why, but I'm annoyed that it's not obvious :/). However, why are nickname and realm being handled differently now? realm in particular is actually part of IUser and should probably remain that way unless there's a good reason to change it.

[David Sturgis]

Itamar and I have made the suggested docstring and whitespace fixes. The new ticket #3664 addresses the MOTD ordering issue. As for nickname and realm, they were required previously, but not set until after login; This new order seems to make more sense. (For example, error messages early in the login process will now contain the correct hostname.)

[z3p]

Conflict when merging w/ trunk.

[David Sturgis]

(In [26276]) Branching to 'pidgin-2385-2'

[David Sturgis]

(In [26277]) Branching to 'pidgin-2385-3'

[David Sturgis]

Merged forward (hopefully correctly this time!)

[Glyph]

Very minor changes:

1. Since the structure of serverInfo is a bit silly (it's a dictionary, but it needs to have specific keys, it should really be a collection of attributes) it should be private pending some better structure that we want to publicly expose. In other words, it should start with a single underscore, _serverInfo.
2. A couple of blank lines have been incorrectly (I assume accidentally?) deleted which aren't really relevant to the implementation, and should be put back in. Merged to trunk, these lines are: service.py:207 service.py:350 service.py:357 service.py:888
3. There is another problem, for which there should be a separate ticket. AWAY isn't implemented on the server, so when I attempt to log in, I see this (thankfully, apparently harmless) traceback:

   2009-02-22 19:26:06-0500 [IRCUser,0,127.0.0.1] Unhandled Error
   	Traceback (most recent call last):
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/python/context.py", line 37, in callWithContext
   	    return func(*args,**kw)
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/internet/selectreactor.py", line 146, in _doReadOrWrite
   	    why = getattr(selectable, method)()
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/internet/tcp.py", line 459, in doRead
   	    return self.protocol.dataReceived(data)
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/words/protocols/irc.py", line 172, in dataReceived
   	    self.handleCommand(command, prefix, params)
   	--- <exception caught here> ---
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/words/protocols/irc.py", line 184, in handleCommand
   	    self.irc_unknown(prefix, command, params)
   	  File "/home/glyph/Projects/Twisted/trunk/twisted/words/protocols/irc.py", line 191, in irc_unknown
   	    raise NotImplementedError(command, prefix, params)
   	exceptions.NotImplementedError: ('AWAY', '', [])
   

Other than that it looks good! Please fix and merge.

[David Sturgis]

serverInfo has been marked private with an underscore, and the blank lines have been unremoved to clean up the diff in r26279. I have entered the AWAY implementation issue as ticket #3668.

[David Sturgis]

(In [26281]) Make twisted.words IRC server compatible with Pidgin.

author: itamar, tenth

reviewer: therve, exarkun, z3p, glyph

Fixes #2385

While there are other issues to be resolved with MOTD and AWAY behaviors as per #3664 and #3668 respectively, twisted.words now always sends an MOTD message, which is required for Pidgin to successfully connect to it. Also, the "nickname" and "realm" attributes are now initialized in time for them to appear in error messages during login.