Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] Raise warnings for json_encode() and json_decode() issues »

[RFC] Raise warnings for json_encode() and json_decode() issues

php.internals

Craig Duncan (PHP)

9 years ago
Hi internals. As my initial thread about introducing warnings to the JSON functions was not immediately flooded with disagreement I took the liberty of creating an RFC for official discussion. The proposal is to have `json_encode()` and `json_decode()` raise warnings whenever a failure occurs, instead of requiring the developer to call `json_last_error()` each time. The functionality of `json_last_error()` and `json_last_error_msg()` are unaffected and they can still be used in exactly the same way they are today https://wiki.php.net/rfc/json_encode_decode_errors Thanks, Craig

Nikita Popov

9 years ago
On Fri, Jul 28, 2017 at 12:59 PM, Craig Duncan <php@duncanc.co.uk> wrote:
> Hi internals. > > As my initial thread about introducing warnings to the JSON functions was > not immediately flooded with disagreement I took the liberty of creating an > RFC for official discussion. > > The proposal is to have `json_encode()` and `json_decode()` raise warnings > whenever a failure occurs, instead of requiring the developer to call > `json_last_error()` each time. > > The functionality of `json_last_error()` and `json_last_error_msg()` are > unaffected and they can still be used in exactly the same way they are > today > > https://wiki.php.net/rfc/json_encode_decode_errors > > Thanks, > Craig >
Operations that are expected to fail should never generate warnings. We should not design functions such that their correct use requires the use of the error suppression operator. I certainly agree that the current situation is not good and that the json functions really ought to be throwing exceptions, but adding a warning now is only going to make this worse. Nikita

Craig Duncan (PHP)

9 years ago
Hi Nikita, Thanks for your input. Would you vote yes for throwing an exception? On 28 July 2017 at 12:07, Nikita Popov <nikita.ppv@gmail.com> wrote:

Jakub Zelenka

9 years ago
On Fri, Jul 28, 2017 at 12:23 PM, Craig Duncan <php@duncanc.co.uk> wrote:
> Hi Nikita, > > Thanks for your input. Would you vote yes for throwing an exception? > >
Just to clarify exceptions are no go for 7.x. It would have to be 8.x and it would be a huge BC break so I'm quite confident that this will fail the vote!

Andrey Andreev

9 years ago
Hi, On Fri, Jul 28, 2017 at 2:39 PM, Jakub Zelenka <bukka@php.net> wrote:
> On Fri, Jul 28, 2017 at 12:23 PM, Craig Duncan <php@duncanc.co.uk> wrote: > >> Hi Nikita, >> >> Thanks for your input. Would you vote yes for throwing an exception? >> >> > Just to clarify exceptions are no go for 7.x. It would have to be 8.x and > it would be a huge BC break so I'm quite confident that this will fail the > vote!
This is a long shot, but here's an idea: an OOP API throwing exceptions. Would require more work both to implement and use, but it would surely be less error-prone and there'd be no BC break. Cheers, Andrey.

Craig Duncan (PHP)

9 years ago
On 28 July 2017 at 12:46, Andrey Andreev <narf@devilix.net> wrote:
> > This is a long shot, but here's an idea: an OOP API throwing exceptions. > > Would require more work both to implement and use, but it would surely > be less error-prone and there'd be no BC break. >
Hi Andrey, Yes that would resolve all BC issues, however that wouldn't help with new developers picking up the `json_*` functions and using them dangerously. As pointed out in the original thread, there are plenty of wrappers that provide this feature, the problem is that core functions have a huge gotcha. If we introduced a new API then we'd need to deprecate the `json_*` functions to make users aware, which I imagine would be the most contentious thing suggested thus far

Andrey Andreev

9 years ago
Hi, On Fri, Jul 28, 2017 at 1:59 PM, Craig Duncan <php@duncanc.co.uk> wrote:
> Hi internals. > > As my initial thread about introducing warnings to the JSON functions was > not immediately flooded with disagreement I took the liberty of creating an > RFC for official discussion. > > The proposal is to have `json_encode()` and `json_decode()` raise warnings > whenever a failure occurs, instead of requiring the developer to call > `json_last_error()` each time. > > The functionality of `json_last_error()` and `json_last_error_msg()` are > unaffected and they can still be used in exactly the same way they are today > > https://wiki.php.net/rfc/json_encode_decode_errors >
I too think that JSON error handling can be a bit tedious at times, but I certainly prefer it that way than having to suppress all json_decode() calls. Cheers, Andrey.

Jakub Zelenka

9 years ago
On Fri, Jul 28, 2017 at 11:59 AM, Craig Duncan <php@duncanc.co.uk> wrote:
> Hi internals. > > As my initial thread about introducing warnings to the JSON functions was > not immediately flooded with disagreement I took the liberty of creating an > RFC for official discussion. > > The proposal is to have `json_encode()` and `json_decode()` raise warnings > whenever a failure occurs, instead of requiring the developer to call > `json_last_error()` each time. > > The functionality of `json_last_error()` and `json_last_error_msg()` are > unaffected and they can still be used in exactly the same way they are > today > > https://wiki.php.net/rfc/json_encode_decode_errors > >
I don't think this is a good idea. Especially in case of json_decode the invalid JSON comes from sources that user does not have any control and can't prevent the fail. Emitting warning wouldn't be helpful and it would just make it more difficult to handle such cases. Also it would break a big amount of code because in case of converting errors to exceptions, you start getting a different exception so it would have to be handled. I would call the whole proposal just a big BC break (I'm aware that we don't consider adding warnings as BC but in this case it is) for no benefit at all! Cheers Jakub

Craig Duncan (PHP)

9 years ago
On 28 July 2017 at 12:36, Jakub Zelenka <bukka@php.net> wrote:
> > Also it would break a big amount of code because in case of converting > errors to exceptions, you start getting a different exception so it would > have to be handled. I would call the whole proposal just a big BC break > (I'm aware that we don't consider adding warnings as BC but in this case it > is) for no benefit at all! > >
Hi Jakub, thanks for sharing you opinion. While I agree there are BC concerns, I don't think it's accurate to say no benefit at all. I regularly see new (and experienced) developers using these functions without checking `json_last_error()`, trying to figure out why the app is failing later without any warnings in the log can be very difficult.