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

Reduce console log spam #3896

Merged
merged 3 commits into from
Nov 20, 2023
Merged

Reduce console log spam #3896

merged 3 commits into from
Nov 20, 2023

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 17, 2023

There's an awful lot going on in our console at the moment, making it hard to find important stuff, and contributing to poor performance and poor log retention.

A couple of different things in this PR:

  • Increase the MaxListeners setting on MatrixClient and Thread, so that we don't get "possible EventEmitter leak" warnings.

  • Disable a couple of warning/info lines that are just part of regular operation and are logged in large volumes.


This change is marked as an internal change (Task), so will not be included in the changelog.

A couple of different things:

 * Increase the `MaxListeners` setting on `MatrixClient` and `Thread`, so that
   we don't get "possible EventEmitter leak" warnings

 * Disable a couple of warnings/info lines that are just part of regular
   operation and are logged in large volumes.
Comment on lines 145 to 146
// client, or malicious behaviour).
logger.warn(`Ignoring receipt for missing event with id ${receipt.eventId}`);
Copy link
Member

Choose a reason for hiding this comment

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

This only just got added - cc @andybalaam

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It's also currently making a test fail, but I'll leave fixing that up until we decide what to do.

EW currently produces a firehose of these warnings, so I'm struggling to believe they are useful.

Copy link
Member

Choose a reason for hiding this comment

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

This usually indicates a bug, unless we got unlucky with a message pointed to by a receipt being deleted.

@florianduros and I have a PR we are hoping to get ready today that fixes 2 bugs that would have caused this message, so please can we tolerate it a little longer until that PR can get merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

Have removed this bit of the PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

this got added back again in #3904

Apparently this is being worked on
@andybalaam
Copy link
Member

Approving this, and we are continuing to discuss the bad receipt message.

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

src/webrtc/ part looks fine to me, that log line has outlived its usefulness

@richvdh
Copy link
Member Author

richvdh commented Nov 20, 2023

thanks all

@richvdh richvdh added this pull request to the merge queue Nov 20, 2023
Merged via the queue into develop with commit 1889f8d Nov 20, 2023
20 checks passed
@richvdh richvdh deleted the rav/less_log_spam branch November 20, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants