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

Posthog | E2EE possible error inconsistency on name cross platform, would need also to add error code in event #224

Closed
BillCarsonFr opened this issue Apr 11, 2022 · 6 comments
Assignees
Labels
A-Telemetry Telemetry / analytics to understand usage Team: Crypto

Comments

@BillCarsonFr
Copy link
Member

BillCarsonFr commented Apr 11, 2022

It looks like web/android/mobile are not using Unspecified vs Unknown the same way. It's suspicious as the graph are inverted for these category on android vs ios; Unspecified looks absent in web?

Also, we need more details regarding unknown/unspecified as they represent a decent chunk of errors. E.g on web nearly all of UTDs are UnknownError

@BillCarsonFr BillCarsonFr added Team: Crypto A-Telemetry Telemetry / analytics to understand usage labels Apr 11, 2022
@BillCarsonFr BillCarsonFr self-assigned this Apr 12, 2022
@BillCarsonFr
Copy link
Member Author

As per discussion we could add more enum values to the error name field, and ensure we have cross platform consistency

UnknownError covers a lot though,

    OLM_MISSING_CIPHERTEXT
    OLM_NOT_INCLUDED_IN_RECIPIENTS
    OLM_BAD_ENCRYPTED_MESSAGE
    OLM_BAD_RECIPIENT
    OLM_BAD_RECIPIENT_KEY
    OLM_FORWARDED_MESSAGE
    OLM_BAD_ROOM
    MEGOLM_MISSING_FIELDS
    OLM_DECRYPT_GROUP_MESSAGE_ERROR
    OLM_UNKNOWN_MESSAGE_INDEX
    MEGOLM_BAD_ROOM

@novocaine
Copy link

novocaine commented Apr 12, 2022

@BillCarsonFr can you post links to the suspicious graphs please? I think the crypto team can action this..

@BillCarsonFr
Copy link
Member Author

I wonder if mobile won't be other reporting the same UISI compared to web.
A mobile app is often killed/start, we don't persist the sessions that were already reported, so everytime you re-open the app we will re-report for that session.
Web is not persisting either but web is often opened for longer.

Is there a way to send the sessionId (or hash) in the error event and deduplicate on posthog?

@duxovni
Copy link

duxovni commented Apr 20, 2022

Would we want to deduplicate on session ID, or on event ID? If we get 12 undecryptable messages from the same user as part of the same megolm session, should that be 1 tracked error or 12?

@duxovni
Copy link

duxovni commented Apr 20, 2022

Web also theoretically has some code to save/load already-reported event IDs from local storage, but it's commented out right now, I forget what the issue was.

@BillCarsonFr
Copy link
Member Author

Consistency now resolved matrix-org/matrix-react-sdk#8916

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Telemetry Telemetry / analytics to understand usage Team: Crypto
Projects
None yet
Development

No branches or pull requests

5 participants