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

Filter out realms connection error message from debug log #991

Merged
merged 2 commits into from
Dec 9, 2023

Conversation

dicedpixels
Copy link
Contributor

@dicedpixels dicedpixels commented Nov 22, 2023

Filter out realms connection error exception message from debug log. Using another <RegexFilter> did not work, hence the current regex.

img

There are two log entries related to not being able to connect to Realms. Only the log entry related to the exception, Couldn't connect to realms is removed.

Sample log attached.

log.txt

@modmuss50
Copy link
Member

You filter out Couldn't connect to realms but in the PR desc you say the error is Could not authorize you against Realms server: Invalid session id. Which one is correct?

@dicedpixels
Copy link
Contributor Author

You filter out Couldn't connect to realms but in the PR desc you say the error is Could not authorize you against Realms server: Invalid session id. Which one is correct?

I've fixed the PR description. What I wanted to highlight is that only the exception Couldn't connect to realms is removed.

Could not authorize you against Realms server: Invalid session id is printed as an info so, I did not remove it.

@dicedpixels
Copy link
Contributor Author

dicedpixels commented Nov 28, 2023

It seems like 1.20.3-pre3 changed the InvalidCredentialsException message. Now it's this:
image

If the way I've done the RegexFilter is okay, I can include this as well.

@modmuss50
Copy link
Member

If the way I've done the RegexFilter is okay, I can include this as well.

Yes, if we can include them all that would be great, shame they keep changing it 😆

Maybe it would be easier to read if there was only 1 message per RegexFilter? Im not too bothered either way.

@dicedpixels
Copy link
Contributor Author

Maybe it would be easier to read if there was only 1 message per RegexFilter? Im not too bothered either way.

I agree too but defining the filters like

<Filters>
  <RegexFilter regex="^Failed to verify authentication$" onMatch="DENY" onMismatch="ACCEPT"/>
  <RegexFilter regex="^Failed to fetch user properties$" onMatch="DENY" onMismatch="ACCEPT"/>
  <RegexFilter regex="^Couldn't connect to realms$" onMatch="DENY" onMismatch="ACCEPT"/>
</Filters>

doesn't seem to work as expected. It filters using the 1st regex and skips the other two. I couldn't find anything explaining this behavior online either. I tried different regexes as well.

@modmuss50
Copy link
Member

Maybe it would be easier to read if there was only 1 message per RegexFilter? Im not too bothered either way.

I agree too but defining the filters like

<Filters>
  <RegexFilter regex="^Failed to verify authentication$" onMatch="DENY" onMismatch="ACCEPT"/>
  <RegexFilter regex="^Failed to fetch user properties$" onMatch="DENY" onMismatch="ACCEPT"/>
  <RegexFilter regex="^Couldn't connect to realms$" onMatch="DENY" onMismatch="ACCEPT"/>
</Filters>

doesn't seem to work as expected. It filters using the 1st regex and skips the other two. I couldn't find anything explaining this behavior online either. I tried different regexes as well.

Ah, thats annoying. Lets just do it as one line then, its not the end of the world.

@dicedpixels
Copy link
Contributor Author

Ah, thats annoying. Lets just do it as one line then, its not the end of the world.

PR updated with @justanothercorpusguy's suggestion and it works.

onMismatch="ACCEPT" indeed stops further processing. NEUTRAL does not.

https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/Filter.Result.html#NEUTRAL

Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

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

Thanks :)

@modmuss50 modmuss50 merged commit 7dfe800 into FabricMC:exp/1.5 Dec 9, 2023
2 checks passed
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