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

[8.x] Improve the exception thrown when JSON encoding response contents fails #36851

Merged
merged 2 commits into from
Apr 2, 2021
Merged

Conversation

LukeTowers
Copy link
Contributor

This improves on the opaque exception of "The Response content must be a string or object implementing __toString(), “boolean” given." that occurs whenever a call to $response->setContent($varThatFailsJsonEncoding) is made. It is difficult to debug the original exception, because to the developer they are not providing a boolean anywhere in their code, they are passing an array or an object to setContent() and expecting it to work; not knowing that Laravel internally tries to json_encode() that value before passing it along to Symfony, which only understands string values as response contents.

Refs:

This improves on the opaque exception of "The Response content must be a string or object implementing __toString(), “boolean” given." that occurs whenever a call to $response->setContent($varThatFailsJsonEncoding) is made. It is difficult to debug the original exception, because to the developer they are not providing a boolean anywhere in their code, they are passing an array or an object to setContent() and expecting it to work; not knowing that Laravel internally tries to json_encode() that value before passing it along to Symfony, which only understands string values as response contents.

Refs:
- https://stackoverflow.com/a/38772790/6652884
- https://octobercms.com/forum/post/error-on-reset-password
- https://octobercms.com/plugin/support/pixel-shop/bug-and-italian-translation
- https://laravelquestions.com/2017/08/16/the-response-content-must-be-a-string-or-object-implementing-__tostring-boolean-given/
- rainlab/user-plugin#294
@LukeTowers
Copy link
Contributor Author

@driesvints if you guys would be willing to backport this to 6.x too, that would be fantastic :)

@crynobone
Copy link
Member

On Laravel 8 why don't we use JSON_THROW_ON_ERROR new feature?

https://stitcher.io/blog/new-in-php-73#json-errors-can-be-thrown-rfc

@driesvints driesvints changed the title Improve the exception thrown when JSON encoding response contents fails [8.x] Improve the exception thrown when JSON encoding response contents fails Apr 2, 2021
@driesvints
Copy link
Member

@LukeTowers nice, thanks for this. I'm not sure about 6.x, I'll let @taylorotwell decide on that.

I also agree that if we introduce exception throwing, the new JSON_THROW_ON_ERROR is probably more applicable here.

@taylorotwell taylorotwell merged commit 1168ec7 into laravel:8.x Apr 2, 2021
@LukeTowers
Copy link
Contributor Author

Yeah, I thought about doing that, but there were too many places where I'd have to add that to trickle up to here and then those places suddenly throwing exceptions could be unexpected behaviour. Plus it's only in 7.3 so it wouldn't work for 6.x

@LukeTowers LukeTowers deleted the patch-1 branch April 2, 2021 16:59
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.

None yet

4 participants