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

Don't fail on insufficient parameters in ParameterFormatter (#2337) #2343

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

vy
Copy link
Member

@vy vy commented Mar 4, 2024

No description provided.

@vy vy added bug Incorrect, unexpected, or unintended behavior of existing code api Affects the public API labels Mar 4, 2024
@vy vy added this to the 2.23.1 milestone Mar 4, 2024
@vy vy requested a review from ppkarwasz March 4, 2024 10:24
@vy vy self-assigned this Mar 4, 2024
Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

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

Personally I would maintain the current logic that throws exceptions, I would just catch them in the call sites.

I looked in Log4j 2.19.0 how formatting was done. Using the StringFormatterMessageFactory a call to logger.info("Hello %s and %s!", "world") gives:

2024-03-04 11:18:57,550 main ERROR Unable to format msg: Hello %s and %s!
java.util.MissingFormatArgumentException: Format specifier '%s'
Hello %s and %s!

whereas the default factories for a call to logger.info("Hello {} and {}!", "world") give just:

Hello world and {}!

I don't think the behavior of Log4j 2.19.0 is correct. I would expect rather:

2024-03-04 11:18:57,550 main ERROR Unable to format msg: Hello {} and {}!
java.lang.IllegalArgumentException: Found 2 argument placeholders, but only 1 was provided.
Hello {} and {}!

Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

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

LGTM

@vy vy merged commit 0eb232f into 2.x Mar 6, 2024
9 checks passed
@vy vy deleted the 2.x-ParameterFormatter-insufficient-args branch March 6, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the public API bug Incorrect, unexpected, or unintended behavior of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants