Skip to content

Print RuntimeException raised by StructuredLogFormatter::format to system err#43371

Closed
quaff wants to merge 1 commit intospring-projects:mainfrom
quaff:patch-106
Closed

Print RuntimeException raised by StructuredLogFormatter::format to system err#43371
quaff wants to merge 1 commit intospring-projects:mainfrom
quaff:patch-106

Conversation

@quaff
Copy link
Contributor

@quaff quaff commented Dec 4, 2024 •
edited
Loading

Before this commit, runtime exception is swallowed silently.

For example, if two StructuredLoggingJsonMembersCustomizer add same key, exception is printed now:

Exception in thread "main" java.lang.IllegalStateException: The name 'test' has already been written
	at org.springframework.util.Assert.state(Assert.java:101)
	at org.springframework.boot.json.JsonValueWriter.writePair(JsonValueWriter.java:228)
	at org.springframework.boot.json.JsonValueWriter.write(JsonValueWriter.java:83)
	at org.springframework.boot.json.JsonWriter$Member.write(JsonWriter.java:650)
	at org.springframework.boot.json.JsonWriter$Members.write(JsonWriter.java:339)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 4, 2024
@philwebb
Copy link
Member

philwebb commented Dec 4, 2024

This is an interesting problem. I'm not sure if JsonWriterStructuredLogFormatter is the best place to fix it. Perhaps org.springframework.boot.logging.logback.StructuredLogEncoder and org.springframework.boot.logging.log4j2.StructuredLogLayout might be better. It's interesting that logback doesn't handle encoder problems directly.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Dec 4, 2024
@philwebb philwebb changed the title Print RuntimeException raised by StructuredLogFormatter::format to system err Print RuntimeException raised by StructuredLogFormatter::format to system err Dec 4, 2024
@quaff
Print RuntimeException raised by StructuredLogFormatter::format to …
ecfdb7f 
…system err

Before this commit, runtime exception is swallowed silently.

For example, if two `StructuredLoggingJsonMembersCustomizer` add same key, exception is printed now:
```
Exception in thread "main" java.lang.IllegalStateException: The name 'test' has already been written
	at org.springframework.util.Assert.state(Assert.java:101)
	at org.springframework.boot.json.JsonValueWriter.writePair(JsonValueWriter.java:228)
	at org.springframework.boot.json.JsonValueWriter.write(JsonValueWriter.java:83)
	at org.springframework.boot.json.JsonWriter$Member.write(JsonWriter.java:650)
	at org.springframework.boot.json.JsonWriter$Members.write(JsonWriter.java:339)
```
@quaff
Copy link
Contributor Author

quaff commented Dec 4, 2024

This is an interesting problem. I'm not sure if JsonWriterStructuredLogFormatter is the best place to fix it. Perhaps org.springframework.boot.logging.logback.StructuredLogEncoder and org.springframework.boot.logging.log4j2.StructuredLogLayout might be better. It's interesting that logback doesn't handle encoder problems directly.

Amended as your suggestion.

@mhalbritter
Copy link
Contributor

mhalbritter commented Dec 4, 2024

I assumed that Logback would handle any exception raised in an encoder. If that's not the case, StructuredLogEncoder and StructuredLogLayout should print them.

@philwebb philwebb self-assigned this Dec 4, 2024
@philwebb philwebb mentioned this pull request Dec 4, 2024
Closed
@philwebb
Copy link
Member

philwebb commented Dec 4, 2024

Thanks for the PR @quaff. Looking into this in more detail, I'm not sure that we should be printing the exception ourselves. It looks like we perhaps should be hooking into more of the logging library features to deal with errors. I've opened #43384 to track that.

@philwebb philwebb closed this Dec 4, 2024
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@philwebb philwebb

Labels

status: superseded An issue that has been superseded by another

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@quaff @philwebb @mhalbritter @spring-projects-issues