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

record, report, and notify about olm errors #1146

Merged
merged 3 commits into from
Jan 15, 2020

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Jan 10, 2020

Implements the no_olm part of MSC2399, and tells the user when the olm session got wedged.

The general algorithm is that when there's a problem with the olm session (either it's wedged, or the sender tells us they can't establish a session), then we try to recover by creating a session ourselves. Then we record the problem, timestamp, and whether we (think we) fixed the problem. When we have a megolm event that we can't decrypt, we check if it was sent within the time that we had a problem, and if so, tell the user what the problem was, and whether we were able to fix it.

@uhoreg uhoreg requested a review from a team January 15, 2020 04:57
@uhoreg uhoreg marked this pull request as ready for review January 15, 2020 04:57
@dbkr dbkr requested review from dbkr and removed request for a team January 15, 2020 11:22
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm I think! I think we do also have stores that are just not implemented in the localstorage / memory stores so you probably could have got away with just writing the indexeddb one and leaving the rest as stubs. People in firefox private mode appreciate your efforts. :)

}
problems.sort((a, b) => {
return a.time - b.time;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw you can have multi-level indices on indexeddb now*, so you would be able to index by timestamp, although not 100% sure if you would be able to get the largest ts lower than the given ts. Hopefully this table won't be big enough that it's a problem in terms of efficiency although if we ever want the index in future then could it be useful to add it now to avoid db changes?

*they might still not be supported on Edge but we already have other tables that use them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did take a look at it, but didn't see an easy way to do the query that I needed.

@uhoreg uhoreg merged commit 9fb4ed2 into matrix-org:develop Jan 15, 2020
@jryans jryans added the phase:1 label Feb 12, 2020
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.

3 participants