tulir / gomuks

A terminal based Matrix client written in Go.
https://maunium.net/go/gomuks
GNU Affero General Public License v3.0
1.34k stars 120 forks source link

Upgrade mautrix go #393

Open n-peugnet opened 1 year ago

n-peugnet commented 1 year ago

Mautrix go has not been updated since a long time. As I wanted to add a feature to the library I noticed that there were quite a lot of breaking changes. So I started by updated mautrix.

I am not sure about the second commit. I added it because after the update, gomuks panicked while loading new messages without relations. I don't know if this is normal, but it seems event.Unsigned.Relations can now be nil, while gomuks expected this variable to always be initialised. If I understand correctly, this value was previously always populated by ParsedRaw(). This new behaviour may be related to this change: https://github.com/mautrix/go/commit/d2005e5dfe54e551ffdab15cebecc373abfd4df4

n-peugnet commented 1 year ago

So the problems indeed come from https://github.com/mautrix/go/commit/d2005e5dfe54e551ffdab15cebecc373abfd4df4. My second commit https://github.com/tulir/gomuks/pull/393/commits/ce4e8674202960225851e14a70c76d27ca369d8c only fixes one of the bugs it introduced in gomuks, as it relies on the Relations structure to always be there in other places.

I tried to revert the Relations part of this commit and it indeed solved the issues.

@tulir: is this breaking change expected or should we try to fix it in mautrix ? If this is expected, then I think it would be good to add the info (about this value being now a pointer and nil if empty) in the release notes of the v0.12.0 release and in the CHANGELOG.md as a breaking change.

Maybe we can keep a pointer and allocate it in mautrix while unmarshalling. But it may defeat the purpose of changing it into a pointer (I am not really sure what was the purpose of changing it into a pointer).

n-peugnet commented 1 year ago

Just upgraded mautrix from v0.12.2 to v0.12.4.

I will keep this version for a few days to see if new problems arise.

n-peugnet commented 1 year ago

Hmm this is not so good. I started to get some "unable to decrypt" errors recently and I suspect it comes from this change.

I noticed in the gomuks debug log the following errors:

[2023-02-02 12:21:30] [Crypto/Error] Error fetching current cross-signing keys of user @tykaynchu:matrix.org: sql: Scan error on column index 2, name "first_seen_key": converting NULL to string is unsupported

And quite a lot of "database is locked" errors:

[2023-02-02 12:21:17] [Crypto/Warn] Failed to store signature from TfqyGCtYHfgQX9OaH/9ABwSOOC4eZQOAd8hVBGs/zUY for qRr++BMA4xL3Nlt9JV4NuVnozd/DQ8Lu2sBKcUdpKO8: database is locked
[2023-02-02 12:21:17] [Crypto/Debug] Storing cross-signing key for @ledan:club1.fr: iW7e+PNCFQLb35rAM7PobtGuMUCy5+NEMcQ0eeYPYu0 (type self_signing)
[2023-02-02 12:21:22] [Crypto/Error] Error storing cross-signing key: database is locked
n-peugnet commented 11 months ago

Hi @tulir, I rebased this branch over master and updated to mautrix v0.14.0 (I will maybe see later for the latest versions). I think the first error described in https://github.com/tulir/gomuks/pull/393#issuecomment-1413584355 came from alternating between v0.11 and v0.12 mautrix versions (which was a bad idea). I don't really know for the second one, but it also might be related. I will keep using this branch for some time to see if these errors come back or if new one arise.