isamert / scli

a simple terminal user interface for signal messenger (using signal-cli)
GNU General Public License v3.0
449 stars 39 forks source link

Empty self._data['groupStore'] causes scli to crash #126

Closed 0mp closed 3 years ago

0mp commented 3 years ago

I get the following error when I start scli:

$ ./scli
Traceback (most recent call last):
  File "./scli", line 4383, in <module>
    main()
  File "./scli", line 4368, in main
    coord = Coordinate()
  File "./scli", line 3742, in __init__
    self._contacts = Contacts(sigdata)
  File "./scli", line 1044, in __init__
    self.reload()
  File "./scli", line 1047, in reload
    indivs_dicts, groups_dicts = self._sigdata.parse_data_file()
  File "./scli", line 893, in parse_data_file
    for g in self._data['groupStore']['groups']:
TypeError: 'NoneType' object is not subscriptable

I could fix it by applying the following patch:

893,900c893,899
<         if self._data['groupStore'] is not None:
<             for g in self._data['groupStore']['groups']:
<                 if is_group_v2(g):
<                     group_id = g['groupId']
<                     g['name'] = self._get_group_v2_cache_name(group_id) or group_id[:10] + '[..]'
<                 if g.get('archived') or not g.get('name'):
<                     continue
<                 groups.append(g)
---
>         for g in self._data['groupStore']['groups']:
>             if is_group_v2(g):
>                 group_id = g['groupId']
>                 g['name'] = self._get_group_v2_cache_name(group_id) or group_id[:10] + '[..]'
>             if g.get('archived') or not g.get('name'):
>                 continue
>             groups.append(g)

but I'm not sure if that's the correct fix.

Some details:

Note: initially, groupStore is empty in ~/.local/share/signal-cli/data/+PHONENUMBERHERE. However, once scli is able to start successfully at least once, then groupStore gets populated just fine.

Note 2: if I start scli with the patch above, I get the following error:

$ ./scli
Traceback (most recent call last):
  File "./scli", line 4384, in <module>
    main()
  File "./scli", line 4378, in main
    loop.run()
  File "/usr/local/lib/python3.8/site-packages/urwid/main_loop.py", line 287, in run
    self._run()
  File "/usr/local/lib/python3.8/site-packages/urwid/main_loop.py", line 385, in _run
    self.event_loop.run()
  File "/usr/local/lib/python3.8/site-packages/urwid/main_loop.py", line 790, in run
    self._loop()
  File "/usr/local/lib/python3.8/site-packages/urwid/main_loop.py", line 827, in _loop
    self._watch_files[fd]()
  File "/usr/local/lib/python3.8/site-packages/urwid/main_loop.py", line 233, in cb
    rval = callback(data)
  File "./scli", line 648, in _daemon_stdout_handler
    self._envelope_handler(envelope)
  File "./scli", line 665, in _envelope_handler
    self.callbacks['receive_sync_message'](envelope)
  File "./scli", line 3832, in _on_receive_sync_message
    self._on_sending_message(envelope)
  File "./scli", line 3768, in _on_sending_message
    group_members = self._contacts.get_by_id(group_id).members_ids
AttributeError: 'NoneType' object has no attribute 'members_ids'

However, the error is gone if I restart scli a couple of times.

exquo commented 3 years ago

Thanks for reporting!

I can reproduce by deleting the groupStore section in signal-cli's data file. Will create a fix soon.

The groupStore being empty on first startup might be related to https://github.com/AsamK/signal-cli/issues/438. Did you link account with scli link or with signal-cli link ..? In the latter case, did you do signal-cli receive afterwards? (scli link would do it automatically).

Just an FYI, signal-cli has just introduced a new sendSyncRequest command that would request your contacts from the master device (presumably re-populating your the local signal-cli data file if they have been removed for whatever reason).
(Note-to-self: if contacts-loss like this is common enough, in the future we could consider executing that command in scli, on startup for example.)
Update: I have tried running sendSyncRequest command after deleting the groupStore, but it did not restore it: https://github.com/AsamK/signal-cli/issues/622

Also, I think you've accidentally included your phone number in the message above (unless it's a "dummy", example number). You can edit your post and then delete the original copy to remove it from history.

0mp commented 3 years ago

Thanks for reporting!

:ok_hand:

I can reproduce by deleting the groupStore section in signal-cli's data file. Will create a fix soon.

Cool, thanks. I'll wait for the new release before I update signal-cli and scli in the FreeBSD Ports. After signal-cli 0.8.2 the whole stack became a bit unstable. But we are getting there :)

The groupStore being empty on first startup might be related to AsamK/signal-cli#438. Did you link account with scli link or with signal-cli link ..? In the latter case, did you do signal-cli receive afterwards? (scli link would do it automatically).

I use scli link.

Also, I think you've accidentally included your phone number in the message above (unless it's a "dummy", example number). You can edit your post and then delete the original copy to remove it from history.

Fixed, thanks for the note.

exquo commented 3 years ago

I've pushed a fix to the develop branch. Could you try it out? If everything works okay, I'll roll it into a new release.

The groupStore being empty on first startup might be related to AsamK/signal-cli#438. Did you link account with scli link or with signal-cli link ..? In the latter case, did you do signal-cli receive afterwards? (scli link would do it automatically).

I use scli link.

Well, that was my only guess.. If you can reliably (or even intermittently) get an empty groupStore after signal-cli link && signal-cli receive (which is in essence what scli link does), that would be a candidate for a new ticket in signal-cli's tracker.

exquo commented 3 years ago

A side question: is it possible to "pin down" a signal-cli version in scli's BSD port to the "latest tested" version in scli's readme?

(I don't use the <= version notation there because it suggests some incompatibility with the later versions, while, first, for most of the time there are no later (untested) versions, and, second, once a new signal-cli version comes out, it usually does not introduce breaking changes in scli)

At the time of new scli releases, the "latest tested" is pretty much always the latest released signal-cli version. However, when there's a new signal-cli version released later, IIUC a user installing scli through Ports would get that latest signal-cli version that might have introduced backwards incompatible changes (like a different files separation in v0.8.2).

Is it possible in Ports to pin a dependency's version for the purposes of a package, while allowing to install the latest version of that dependency as a package in its own right? (kinda like Python's pip and virtualenv) For those who want the both to use scli and have the latest signal-cli version for other purposes.

Now that I think about, I'm not sure how to go about updating that dependency version in Ports without too much manual effort. It does not feel right to create a new release just because "a new version of a dependency came out and it did not break our program!" :). But it also does not sound like fun for you to have to watch the scli commits for "bump the tested signal-cli version" messages. So maybe this is not such a hot idea after all.. The way it's done now (test new versions soon after they come out; if smth breaks - fix and make a new release) works well enough.

0mp commented 3 years ago

I've pushed a fix to the develop branch. Could you try it out? If everything works okay, I'll roll it into a new release.

Works just fine. I've also relinked the device to see if a freshly linked device is also working. It seems to be ready for prime time. Thank you!

The groupStore being empty on first startup might be related to AsamK/signal-cli#438. Did you link account with scli link or with signal-cli link ..? In the latter case, did you do signal-cli receive afterwards? (scli link would do it automatically).

I use scli link.

Well, that was my only guess.. If you can reliably (or even intermittently) get an empty groupStore after signal-cli link && signal-cli receive (which is in essence what scli link does), that would be a candidate for a new ticket in signal-cli's tracker.

I'll report it upstream if I happen to find a way to reproduce it.

A side question: is it possible to "pin down" a signal-cli version in scli's BSD port to the "latest tested" version in scli's readme?

(I don't use the <= version notation there because it suggests some incompatibility with the later versions, while, first, for most of the time there are no later (untested) versions, and, second, once a new signal-cli version comes out, it usually does not introduce breaking changes in scli)

At the time of new scli releases, the "latest tested" is pretty much always the latest released signal-cli version. However, when there's a new signal-cli version released later, IIUC a user installing scli through Ports would get that latest signal-cli version that might have introduced backwards incompatible changes (like a different files separation in v0.8.2).

Is it possible in Ports to pin a dependency's version for the purposes of a package, while allowing to install the latest version of that dependency as a package in its own right? (kinda like Python's pip and virtualenv) For those who want the both to use scli and have the latest signal-cli version for other purposes.

Yes and no. The FreeBSD Ports infrastructure allows for requiring specific versions of ports. E.g., the scli port may require signal-cli to be of a very specific version. However, if someone decides to commit an update to signal-cli, then the scli port will refuse to build due to missing dependencies.

Usually, programs keep backward compatibility so it's safe to just update them. However, in cases where a very specific, legacy version of port is needed, then we just create a new port in the FreeBSD Ports Collection (see, e.g., http://freshports.org/sysutils/ansible and https://www.freshports.org/sysutils/ansible27).

In theory, we could create a special signal-cli port just for scli so that scli never breaks due to an update to scli. In practice, however, I am the maintainer of both ports and I make sure that ports dependent on signal-cli do not break when I update it. And that's generally how we do it in FreeBSD.

Now that I think about, I'm not sure how to go about updating that dependency version in Ports without too much manual effort. It does not feel right to create a new release just because "a new version of a dependency came out and it did not break our program!" :).

Haha, I see your point.

But it also does not sound like fun for you to have to watch the scli commits for "bump the tested signal-cli version" messages. So maybe this is not such a hot idea after all.. The way it's done now (test new versions soon after they come out; if smth breaks - fix and make a new release) works well enough.

This model seems pretty simple and works rather well and since signal-cli's interface is not very stable, this may be the best workflow for now.

exquo commented 3 years ago

This model seems pretty simple and works rather well and since signal-cli's interface is not very stable, this may be the best workflow for now.

Agreed!

Well, I've learnt a bit about BSD ports today :)

0mp commented 3 years ago

Well, I've learnt a bit about BSD ports today :)

Happy to help :)