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

Upgrade mautrix go #393

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

n-peugnet
Copy link
Contributor

@n-peugnet n-peugnet commented Oct 26, 2022

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: mautrix/go@d2005e5

@n-peugnet
Copy link
Contributor Author

n-peugnet commented Oct 27, 2022

So the problems indeed come from mautrix/go@d2005e5. My second commit ce4e867 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
Copy link
Contributor Author

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 n-peugnet marked this pull request as ready for review January 12, 2023 18:59
@n-peugnet
Copy link
Contributor Author

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

Only needed to use NewContext instead of make
@n-peugnet
Copy link
Contributor Author

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 #393 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant