keiffster / program-y

Python 3.x based AIML 2.0 Chatbot interpreter, framework, related programs and knowledge files
https://keiffster.github.io/program-y/
Other
348 stars 138 forks source link

XMPP client JID handling problem #181

Closed harpsdm closed 5 years ago

harpsdm commented 5 years ago

The XMPP client passes a JID object around as the user ID and that leads to multiple problems that prevents the bot from responding to an XMPP message.

Current Behavior

I see multiple TypeErrors logged of the form: TypeError: dharper@xxxxxx.com/jabber_138572337 is not JSON serializable Also the bot fails to respond to the jabber request. See detailed description for more information.

Possible Solution

It appears to me that passing a JID object as the userid is too much information for pprogram-y, and really all that is required is a string. So the following change should resolve the problem:

diff --git a/src/programy/clients/polling/xmpp/xmpp.py b/src/programy/clients/polling/xmpp/xmpp.py index dd77d98..2d6a167 100644 --- a/src/programy/clients/polling/xmpp/xmpp.py +++ b/src/programy/clients/polling/xmpp/xmpp.py @@ -61,7 +61,7 @@ class XmppClient(sleekxmpp.ClientXMPP):

         return msg['body']

     def get_userid(self, msg):
-        return msg['from']
+        return msg['from'].bare

     def send_response(self, msg, response):
         if response is not None:

In my testing, this seems to be working ok, though my testing hasn't been extensive.

Steps to Reproduce

  1. Checkout a copy of the current program-y and y-bot top of the tree
  2. Run the XMPP client. I'm using xmpp.co as my jabber service for the bot, and using the Cisco Jabber client on the Cisco Webex backend as the remote user. I suspect that this bit is important since different clients and servers will pass different amounts of information back and forth.
  3. Send any message to the bot.

Context (Environment)

Detailed Description

I'm seeing a couple of related issues with the XMPP client's handling of the remote Jabber ID.

The first of these is a bunch of TypeErrors that occur when processing an incoming message. The errors are due to the JID object not being handled by json.dumps(). E.g.:

File "/home/dharper/k9brain2/program-y/src/programy/storage/stores/file/store/conversations.py", line 47, in store_conversation json_text = json.dumps(convo_json, indent=4) File "/usr/lib/python3.5/json/init.py", line 237, in dumps **kw).encode(obj) File "/usr/lib/python3.5/json/encoder.py", line 200, in encode chunks = list(chunks) File "/usr/lib/python3.5/json/encoder.py", line 429, in _iterencode yield from _iterencode_dict(o, _current_indent_level) File "/usr/lib/python3.5/json/encoder.py", line 403, in _iterencode_dict yield from chunks File "/usr/lib/python3.5/json/encoder.py", line 403, in _iterencode_dict yield from chunks File "/usr/lib/python3.5/json/encoder.py", line 436, in _iterencode o = _default(o) File "/usr/lib/python3.5/json/encoder.py", line 179, in default raise TypeError(repr(o) + " is not JSON serializable") TypeError: dharper@xxxxxx.com/jabber_138572337 is not JSON serializable

From what I can see, you are really only looking for a string here rather than a JID object, so a pontential fix is to send the JID.bare value rather than the JID (see proposed code change above). I'm suggesting the use of 'bare' here rather than 'full', because there is a separate problem that occurs when the JID has a resourcepart. When trying to write the conversation log, the / that separates the resource part gets interpreted as part of the file path and you get a different error:

2019-01-01 16:38:51,449 root DEBUG - Writing conversation to [y-bot/storage/conversations/XMPP_dharper@xxxxxx.com/jabber_138572337.conv] 2019-01-01 16:38:51,449 root ERROR - Failed to write conversation file [y-bot/storage/conversations/XMPP_dharper@xxxxxx.com/jabber_138572337.conv] [[Errno 2] No such file or directory: 'y-bot/storage/conversations/XMPP_dharper@xxxxxxx.com/jabber_138572337.conv'] 2019-01-01 16:38:51,449 root ERROR - Traceback (most recent call last): Traceback (most recent call last): File "/home/dharper/k9brain2/program-y/src/programy/storage/stores/file/store/conversations.py", line 50, in store_conversation with open(conversation_filepath, "w+") as convo_file: FileNotFoundError: [Errno 2] No such file or directory: 'y-bot/storage/conversations/XMPP_dharper@xxxxxxx.com/jabber_138572337.conv' 2019-01-01 16:38:51,450 root ERROR - File "/home/dharper/k9brain2/program-y/src/programy/storage/stores/file/store/conversations.py", line 50, in store_conversation with open(conversation_filepath, "w+") as convo_file: Traceback (most recent call last): File "/home/dharper/k9brain2/program-y/src/programy/storage/stores/file/store/conversations.py", line 50, in store_conversation with open(conversation_filepath, "w+") as convo_file: FileNotFoundError: [Errno 2] No such file or directory: 'y-bot/storage/conversations/XMPP_dharper@xxxxxx.com/jabber_138572337.conv' 2019-01-01 16:38:51,450 root ERROR - FileNotFoundError: [Errno 2] No such file or directory: 'y-bot/storage/conversations/XMPP_dharper@xxxxxxx.com/jabber_138572337.conv' Traceback (most recent call last): File "/home/dharper/k9brain2/program-y/src/programy/storage/stores/file/store/conversations.py", line 50, in store_conversation with open(conversation_filepath, "w+") as convo_file: FileNotFoundError: [Errno 2] No such file or directory: 'y-bot/storage/conversations/XMPP_dharper@xxxxxxx.com/jabber_138572337.conv' 2019-01-01 16:38:51,450 sleekxmpp.xmlstream.xmlstream DEBUG - SEND: connect140219423Greetings!

By using 'bare' instead of 'full', the resourcepart is omitted and this problem avoided. Granted you won't be able to distinguish when the remote users switches between XMPP endpoints, but I doubt that is a big issue.

keiffster commented 5 years ago

Thanks, that might explain a couple of things. I built the XMPP client to work with hangouts initially and never tried a jabber client during dev. I tried it once but it didn’t work and just put it down to a misconfigired jabber server. I’ll add you change in the next release. Thanks again

harpsdm commented 5 years ago

No worries. Happy to help.

keiffster commented 5 years ago

This is now fixed and new version pushed to master