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

Skipping audit logs against bot message deletions #395

Merged
merged 2 commits into from
Feb 28, 2022

Conversation

Zabuzard
Copy link
Member

@Zabuzard Zabuzard commented Feb 22, 2022

Overview

Implements and closes #393.

This adds a little check that skips writing audit log messages for message deletions against bots. Cause they generate noise that doesnt need to be reviewed and we want the channel to be noise-free. I.e. logs like this are gone now:

message deletion against bot entry

Details

The code changes are not trivial but quite straightforward. Unfortunately, the setup didnt allow a simple if-else, cause at the point where we know the target user, we are already in an async chain of RestAction.

I had to bubble up the target-user information one level from the more generic handleAction to the place where we have the context of MESSAGE_DELETION, i.e. handleMessageDeleteEntry. Therefore, I decided to create a little record that basically holds the User target and the MessageEmbed that handleAction used to create. I was able to enhance this further by letting the record actually generate the embed, moving the code from one place into the other.

Now that the embed and target is known at handleMessageDeleteEntry, we can check for target.isBot() and abort the computation. Unfortunately, I was not able to find any way to gracefully stop/cancel/fail a RestAction other than actually throwing (there is addCheck, but it has no hands on the value from the RestAction, so it cant be used for this) (@Tais993 any better ideas?).

I found a exception in the JDK that seems fitting for this purpose and reused it, CancellationException.

Checklist

  • Implement
  • Absorb this exception, it shouldnt lead to logs and noise

@Tais993
Copy link
Member

Tais993 commented Feb 22, 2022

Nope, you have the correct solution.
Another solution is to use CompletaebleFuture's, but I'm not a fan of that since they'll over-complicate the code a lot (imho)

A second note, you're throwing, this means that exception gets throw and gets logged.
We could set a default failure consumer(RestAction#setDefaultConsumer), which logs CancellationExceptions at warning instead of error.

@Zabuzard
Copy link
Member Author

Nope, you have the correct solution. Another solution is to use CompletaebleFuture's, but I'm not a fan of that since they'll over-complicate the code a lot (imho)

A second note, you're throwing, this means that exception gets throw and gets logged. We could set a default failure consumer(RestAction#setDefaultConsumer), which logs CancellationExceptions at warning instead of error.

Good point. Ill have a look at that. Its unfortunately quite tricky to test this due to the audit log rate limitting. Maybe I can mock it quickly for testing it out.

@Zabuzard Zabuzard force-pushed the feature/skip_audit_bot_message_deletions branch from 3fedf97 to ef0d97c Compare February 24, 2022 08:44
@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Zabuzard Zabuzard merged commit 7ba661d into develop Feb 28, 2022
@Zabuzard Zabuzard deleted the feature/skip_audit_bot_message_deletions branch February 28, 2022 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dont audit-log message deletions against bots
3 participants