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

Added JSON_PARTIAL_OUTPUT_ON_ERROR to the default json encoding options #1515

Closed
wants to merge 1 commit into from

Conversation

zerkms
Copy link
Contributor

@zerkms zerkms commented Nov 26, 2020

Especially when monolog logs the exception traces (including arguments) there is a chance one of the arguments would contain recursive references, which json_encode by default cannot handle.

JSON_PARTIAL_OUTPUT_ON_ERROR option turns those recursive references to null instead.

It's available from php 5.5 hence I think should be okay to be added by default.

Especially when monolog logs the exception traces (including arguments) there is a chance one of the arguments would contain recursive references, which `json_encode` by default cannot handle.

`JSON_PARTIAL_OUTPUT_ON_ERROR` option turns those recursive references to `null` instead.

It's available from php 5.5 hence I think should be okay to be added by default.
@zerkms
Copy link
Contributor Author

zerkms commented Nov 26, 2020

Okay, I realised that it breaks the corresponding test: testIgnoresRecursiveObjectReferences

So please hold on then.

The question then is: would the project want to have it at all? (then I will fix tests and write more if necessary)

@Seldaek Seldaek changed the base branch from master to main December 9, 2020 22:34
@Seldaek Seldaek closed this in 9ce0431 Dec 10, 2020
@Seldaek
Copy link
Owner

Seldaek commented Dec 10, 2020

I think it makes sense yeah, we rather keep as much data as possible in there, fixed the tests quickly.

@Seldaek
Copy link
Owner

Seldaek commented Dec 10, 2020

Thanks for the tip, didn't realize this was there somehow :)

@Seldaek Seldaek added this to the 2.x milestone Dec 10, 2020
@zerkms zerkms deleted the patch-1 branch December 10, 2020 20:49
@lyrixx lyrixx mentioned this pull request Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants