hfaran / slack-export-viewer

A Slack Export archive viewer that allows you to easily view and share your Slack team's export
https://pypi.python.org/pypi/slack-export-viewer
MIT License
966 stars 193 forks source link

Do not crash and burn if "replies" is missing "user" or "ts" entry #167

Closed rusq closed 1 year ago

rusq commented 1 year ago

Hey @hfaran ,

One of the Slackdump users bumped into this issue, where the Slack API does not return the complete "replies" section for a message. Full details are available in https://github.com/rusq/slackdump/issues/222

In brief, this is how the error looks like

  File "C:\Users\User\.local\pipx\venvs\slack-export-viewer\Lib\site-packages\slackviewer\reader.py", line 189, in _create_messages
    chats = self._build_threads(chats)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\User\.local\pipx\venvs\slack-export-viewer\Lib\site-packages\slackviewer\reader.py", line 230, in _build_threads
    item_lookup_key = (item['user'], item['ts'])
                       ~~~~^^^^^^^^
KeyError: 'user'

This happens because one or more messages have incomplete "replies" value, i.e.:

    "replies": [
      {
        "ts": "1579562553.009200"
      }
],

While it should look like this:

    "replies": [
      {
        "user": "US6CXSEBU",
        "ts": "1579562553.009200"
      }
    ],

This causes _build_threads to panic.

Possible improvements

This is a "quick" fix. Maybe it would make sense to log the username, or the message ID if that happens, so that the user could troubleshoot. Let me know if that's what you'd be interested in and I could spend some more time on this.

Thank you for looking into this.

r-belounis commented 1 year ago

For the moment the only way to "trick" the build for me, was to change item['user] to item.get('user'). Solution comes from this thread : https://github.com/hfaran/slack-export-viewer/issues/128#issuecomment-635441531

This is working only if build is stuck. This is not a solution of course, another quick fix only.

rusq commented 1 year ago

Thanks for merging @hfaran !

@r-belounis not sure what you're trying to say tbh, this is just the general dictionary access pattern, if you are expecting that there are missing keys and you're not handling the KeyError exception, but the issue you mentioned is of the same kind, that's correct.