Skip to content

Conversation

@michalsn
Copy link
Member

@michalsn michalsn commented Nov 5, 2025

Description
This PR fixes JSON encoding failures when exception traces contain resources (like database connections), closures, or circular references in API responses.

Additionally, it updates the JSON response trace details to match those of traditional HTML-based error pages. Previously, the JSON traces were overly simplified because json_encode() only serializes public object properties, ignoring private and protected ones.

I tried to find out whether this simplified JSON trace was intentional, but couldn't find any clear reason for it.

Fixes #9786

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 5, 2025
@neznaika0
Copy link
Contributor

neznaika0 commented Nov 6, 2025

Where are private properties used? Did I get it wrong?

What do you think about putting it in a separate Normalizer class? Because the same action is sometimes necessary for Response. Yes, in this case it is better to take care of the types earlier, but if you just return the object from the Formatter, it will also be a mistake. Besides, we have other XML formatter,

@michalsn
Copy link
Member Author

michalsn commented Nov 6, 2025

Where are private properties used? Did I get it wrong?

If your class has private properties, they will appear in the args list just like any others. The same happens in the HTML-based error exception.

What do you think about putting it in a separate Normalizer class? Because the same action is sometimes necessary for Response. Yes, in this case it is better to take care of the types earlier, but if you just return the object from the Formatter, it will also be a mistake. Besides, we have other XML formatter,

I don't really see the point. I don't think any of these should be exposed through our API. In such cases, I would expect an error to be raised to indicate the issue. And adding extra processing to every API response is not something I would like.

Copy link
Contributor

@neznaika0 neznaika0 left a comment

Choose a reason for hiding this comment

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

Good. Important questions should be asked right away. When we look at PR a year later, it's hard to remember why this was done.

@michalsn
Copy link
Member Author

michalsn commented Nov 6, 2025

When we look at PR a year later, it's hard to remember why this was done.

Well, true... but it usually happens to me after like 2 weeks 😅

@michalsn michalsn merged commit 9972ca0 into codeigniter4:develop Nov 6, 2025
50 checks passed
@michalsn
Copy link
Member Author

michalsn commented Nov 6, 2025

Thank you everyone for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: When throwing an exception as JSON, the JSON encoder encounters a type error.

4 participants