Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyError: 'flags' #920

Closed
klfoulk16 opened this issue Feb 13, 2021 · 3 comments · Fixed by #1162
Closed

KeyError: 'flags' #920

klfoulk16 opened this issue Feb 13, 2021 · 3 comments · Fixed by #1162
Labels
bug Something isn't working high priority should be done as soon as possible
Milestone

Comments

@klfoulk16
Copy link
Collaborator

klfoulk16 commented Feb 13, 2021

Screen Shot 2021-02-12 at 8 38 22 PM

The following error was in my zulip-terminal-thread-exceptions.log:

Traceback (most recent call last):
File "/Users/kellyfoulk/Documents/code/zulip-terminal/zulipterminal/model.py", line 1265, in poll_for_events
self.event_actionsevent['type']
File "/Users/kellyfoulk/Documents/code/zulip-terminal/zulipterminal/model.py", line 1124, in _handle_update_message_flags_event
if flag_to_change not in msg['flags']:
KeyError: 'flags'

I was not doing anything in the terminal when this appeared. I had been on my browser and when I moved back to the terminal this was showing on the screen. This was about 1.5 hours after I had issue #919 occur and it was within the same zulip-terminal instance.

Running Zulip Terminal 0.6.0+git on mac in a Z shell

@neiljp neiljp added the bug Something isn't working label Feb 14, 2021
@dehnert
Copy link

dehnert commented Apr 2, 2021

I also hit this, but don't know why. (Zulip-Terminal in screen in Gnome Terminal, checked out from git today.)

@zee-bit
Copy link
Member

zee-bit commented Jun 18, 2021

Bumping the priority, since I happened to encounter the same bug twice today. Trying to figure out a quick fix too...
We might be able to prevent the error by simply adding the field, if not present already, in _handle_update_message_event, but I am more interested in exploring the cause of the bug (or be able to reproduce it) before actually going for a "supposed" fix.

@zee-bit zee-bit added the high priority should be done as soon as possible label Jun 18, 2021
@neiljp
Copy link
Collaborator

neiljp commented Mar 11, 2022

After a little debugging, I can now reproduce this via:

  • open ZT
  • use another client (eg. webapp) to react to a message not currently cached by ZT
  • again outside of ZT, star/unstar that same message

A small fix appears to resolve this well, and I'm going to do a small PR and merge shortly, though we may want to consider our use of defaultdict more generally, at least in certain datastructures.

@neiljp neiljp added this to the Next Release milestone Mar 11, 2022
neiljp added a commit to neiljp/zulip-terminal that referenced this issue Mar 11, 2022
This updates an approach taken in the original reaction implementation.

Since locally cached messages are currently stored in a `defaultdict`,
direct indexing with a non-present message-id creates an empty message.
Such messages then generate `KeyError`s when flags are updated later
(eg. marking read, toggling star status), if the message has not been
fetched.

Fixes zulip#920.
neiljp added a commit to neiljp/zulip-terminal that referenced this issue Mar 11, 2022
This updates an approach taken in the original reaction implementation.

Since locally cached messages are currently stored in a `defaultdict`,
direct indexing with a non-present message-id creates an empty message.
Such messages then generate `KeyError`s when flags are updated later
(eg. marking read, toggling star status), if the message has not been
fetched.

Tests updated to explicitly exclude generation of empty index for
reaction events.

Fixes zulip#920.
neiljp added a commit that referenced this issue Mar 11, 2022
This updates an approach taken in the original reaction implementation.

Since locally cached messages are currently stored in a `defaultdict`,
direct indexing with a non-present message-id creates an empty message.
Such messages then generate `KeyError`s when flags are updated later
(eg. marking read, toggling star status), if the message has not been
fetched.

Tests updated to explicitly exclude generation of empty index for
reaction events.

Fixes #920.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority should be done as soon as possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants