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

JSON::GeneratorError expose invalid object #712

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

byroot
Copy link
Member

@byroot byroot commented Nov 25, 2024

Fix: #710

Makes it easier to debug why a given tree of objects can't be dumped as JSON.

Fix: ruby#710

Makes it easier to debug why a given tree of objects can't
be dumped as JSON.

Co-Authored-By: Étienne Barrié <[email protected]>
@byroot byroot merged commit dbd5042 into ruby:master Nov 25, 2024
36 checks passed
@byroot byroot deleted the generation-error branch November 25, 2024 12:19
if @invalid_object.nil?
super
else
"#{super}\nInvalid object: #{@invalid_object.inspect}"
Copy link
Contributor

Choose a reason for hiding this comment

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

The only quibble I've run into when pulling this into Sidekiq is the newline here. It's injecting formatting into the message, typically if I want to log an error message I don't expect it to take multiple lines in the log output. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course we expect newlines with backtraces... Could exceptions support a hash of context values to be logged with the error, where invalid_object would already be there? This is pretty standard stuff for structured logging, e.g. Golang.

rescue JSON::GeneratorError => ex
  logger.info { ex.with_context(some: value, another: value).detailed_message }

I guess the bigger question would be if ruby-core wants to move toward more formalized structured logging, by providing these types of APIs.

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.

Better error debugging for generation
2 participants