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

key error in helper.py on start #694

Closed
ExpandingMan opened this issue Jun 17, 2020 · 7 comments · Fixed by #706
Closed

key error in helper.py on start #694

ExpandingMan opened this issue Jun 17, 2020 · 7 comments · Fixed by #706
Labels
bug Something isn't working high priority should be done as soon as possible
Milestone

Comments

@ExpandingMan
Copy link

On starting zulip-term I get the following error

ERROR:root:225542
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/zulipterminal/cli/run.py", line 296, in main
    Controller(zuliprc_path,
  File "/usr/lib/python3.8/site-packages/zulipterminal/core.py", line 43, in __init__
    self.model = Model(self)
  File "/usr/lib/python3.8/site-packages/zulipterminal/model.py", line 115, in __init__
    self.unread_counts = classify_unread_counts(self)
  File "/usr/lib/python3.8/site-packages/zulipterminal/helper.py", line 414, in classify_unread_counts
    if [model.stream_dict[stream_id]['name'],
KeyError: 225542

As you can see, this is with Python 3.8. This is on manjaro with zulip-term 0.5.1 installed via pip.

@neiljp neiljp added bug Something isn't working high priority should be done as soon as possible labels Jun 17, 2020
@neiljp neiljp added this to the 0.5.2 milestone Jun 17, 2020
@neiljp
Copy link
Collaborator

neiljp commented Jun 17, 2020

@ExpandingMan Do you have a lot of streams/organizations on your server? Is this on zulipchat.com? That's a high stream id!

Regarding the specific bug, my suspicion is that we're not handling the case where you were initially subscribed, gained unread messages, then unsubscribed, so the unread details don't appear in that dictionary.

As a workaround, if my suspicions are correct: if you recently unsubscribed, one option may be to just re-subscribe, if that's feasible. If you don't wish to stay subscribed, you might mark everything as read and quickly unsubscribe (in eg. webapp). Of course, if you wish to keep your unread counts then that's not going to work for you.

Let us know if you use the workaround to any satisfaction, as it would also go some way to confirming the source of the issue.

@ExpandingMan
Copy link
Author

If the stream ID's are ordered rather than hashed, then I suspect the stream ID itself is in error. This is on julialang.zulipchat.com, I don't think we have a particularly high number of streams.

If I'm understanding what you're saying correctly, it sounds like there is some stream which I am unsubscribed from on which I have unread messages and that you think this somehow leads to the error. While that may well be, unfortunately I am not even sure which streams I had previously subscribed to but since unsubscribed from. I therefore can't implement this workaround unless I have some way of seeing that.

@neiljp
Copy link
Collaborator

neiljp commented Jun 18, 2020

While there may not be many streams on the julialang zulip, the ids could be higher since this is on zulipchat.com, by my understanding.

I'm not sure how comfortable you are with the source and/or python, but from the code this would be in the Model, in self.initial_data['unsubscribed'], though I haven't worked with that data significantly. If you did a print(...) of that near the end of the Model __init__ then you may find it out. You may be able to determine something similar from inspecting network traffic or similar when the webapp connects. Otherwise, I'm afraid we'll just have to fix the bug - I've marked it as important for our next release!

@ExpandingMan
Copy link
Author

I was going to hop in to it with a debugger, look what's going on and let you know. Unfortunately, I'm finding Python environments to be just as irritating as ever and simply running from the cloned repo without doing anything else resulted in a rather spectacular crash of the entirety of X windows that required a display server restart, so not quite sure what happens there.

Anyway, most likely I'll wait for you guys to do some sort of patch and then check it again. Thanks!

@neiljp
Copy link
Collaborator

neiljp commented Jun 18, 2020

Oh wow, that's not good! Maybe down to the debugger using lots of memory? Just a guess.

@ExpandingMan
Copy link
Author

No nothing like that, something with alacritty goes haywire I think. Might be worth trying again, I wouldn't worry about it. The key error is a clear problem, this probably isn't.

neiljp added a commit to neiljp/zulip-terminal that referenced this issue Jul 2, 2020
Unread count data from the server may include that for unsubscribed
streams, which currently are not indexed.

Fixes zulip#694.
neiljp added a commit to neiljp/zulip-terminal that referenced this issue Jul 2, 2020
Unread count data from the server may include that for unsubscribed
streams, which currently are not stored in Model.stream_dict.

Fixes zulip#694.
neiljp added a commit to neiljp/zulip-terminal that referenced this issue Jul 2, 2020
Unread count data from the server may include that for unsubscribed
streams, which currently are not stored in Model.stream_dict.

In the webapp (zulip/zulip repo), stream data includes unsubscribed and
'never_subscribed' streams (see populate_subscriptions in
stream_data.js), so this issue does not arise.

This has been an increased or particular issue recently due to addition
of support for moving messages between streams.

Fixes zulip#694.
neiljp added a commit that referenced this issue Jul 3, 2020
Unread count data from the server may include that for unsubscribed
streams, which currently are not stored in Model.stream_dict.

In the webapp (zulip/zulip repo), stream data includes unsubscribed and
'never_subscribed' streams (see populate_subscriptions in
stream_data.js), so this issue does not arise.

This has been an increased or particular issue recently due to addition
of support for moving messages between streams.

Fixes #694.
@neiljp
Copy link
Collaborator

neiljp commented Jul 3, 2020

@ExpandingMan #706 should be a simple fix for this and is in the main branch now - it will be in 0.5.2, but if you wish to test the development version and see if this fixes the issue for you too this would be useful to know. Thanks again for reporting!

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.

2 participants