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

Add some logging around sync for stuck message debugging #1300

Closed
wants to merge 3 commits into from

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Apr 3, 2020

For matrix-org/synapse#7206

Reviewer: if this looks okay, please merge it so the backend team can test with it ASAP.

Intended to be removed (or possibly converted into a proper off-by-default setting) once the issue concludes.

There's a bunch of null checking through defaults here because this is a very critical part of the whole stack. We default to writing errors to the console, but do not break the app for failing to log info about the sync.

To enable, run localStorage.setItem("mx_do_sync_logging", "true"); in the JS console

For matrix-org/synapse#7206

Intended to be removed (or possibly converted into a proper off-by-default setting) once the issue concludes.

There's a bunch of null checking through defaults here because this is a very critical part of the whole stack. We default to writing errors to the console, but do not break the app for failing to log info about the sync.

This may have adverse affects on performance for large accounts - use `localStorage.setItem("mx_skip_sync_logging", "true")` to disable the worst parts.
For tests this was adding 25 minutes before giving up.
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Seems safe to land as a debugging tool, thanks!

src/sync.js Outdated Show resolved Hide resolved
@turt2live turt2live requested a review from jryans April 3, 2020 15:36
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Tests seem still busted: TypeError: this._currentSyncRequest.abort is not a function

@turt2live
Copy link
Member Author

Looks like we're just going to have to give up on this and debug somehow else (like having devtools open to consume all the RAM)

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.

2 participants