Everything

php

[RFC] [Discussion] JSON_THROW_ON_ERROR

php.internals

Andrew Faulds

8 years ago

Hi everyone,

Craig Duncan previously sparked discussion here about JSON's
error-handling behaviour. Unfortunately, his attempt to change it seems
not to have gone anywhere, but I have his blessing to try this other
approach instead.

So here's an RFC: https://wiki.php.net/rfc/json_throw_on_error

The feature can be described in a single paragraph (in fact, the title
is pretty much enough, the patch is just detail) but it's better to go
through the proper RFC process.

Please tell me what you think.

Thanks!

--
Andrea Faulds
https://ajf.me/

Bradley Weston

8 years ago

I definitely hate having to test the return value and then getting the
error message via a function call. +1 for ability to throw exceptions
instead.


On 10/09/17 02:24, Andrea Faulds wrote:

Jakub Zelenka

8 years ago

On Sun, Sep 10, 2017 at 2:24 AM, Andrea Faulds <ajf@ajf.me> wrote:

> Hi everyone,
>
> Craig Duncan previously sparked discussion here about JSON's
> error-handling behaviour. Unfortunately, his attempt to change it seems not
> to have gone anywhere, but I have his blessing to try this other approach
> instead.
>
> So here's an RFC: https://wiki.php.net/rfc/json_throw_on_error
>
> The feature can be described in a single paragraph (in fact, the title is
> pretty much enough, the patch is just detail) but it's better to go through
> the proper RFC process.
>
> Please tell me what you think.
>
>

I think that it makes sense in general. There are just couple of things
that are not described in RFC.

Currently it has higher priority than JSON_PARTIAL_OUTPUT_ON_ERROR. I think
that if someone specify this constant, then the partial output is required
and it should be preferred. In any case I think it should be in the RFC
what the behavior is.

Another thing is the logic of setting global json error. Currently it just
reset the error value so the next call to json_last_error() will return 0
(no error) even though there was an error which is in the exception code. I
think that the behavior should be either not resetting the global error
value at all (the previous error should be kept) or setting the global
error value so the json_last_error returns the same value as exception code
or 0 if no error. Again it would be good to mention the behavior in the RFC
so we prevent a new discussion in the PR... :)

Cheers

Andrew Faulds

8 years ago

Hi Jakub,

I already replied to you off-list, but for the benefit of people on-list…

Jakub Zelenka wrote:

> On Sun, Sep 10, 2017 at 2:24 AM, Andrea Faulds <ajf@ajf.me> wrote:
>
>> Hi everyone,
>>
>> Craig Duncan previously sparked discussion here about JSON's
>> error-handling behaviour. Unfortunately, his attempt to change it seems not
>> to have gone anywhere, but I have his blessing to try this other approach
>> instead.
>>
>> So here's an RFC: https://wiki.php.net/rfc/json_throw_on_error
>>
>> The feature can be described in a single paragraph (in fact, the title is
>> pretty much enough, the patch is just detail) but it's better to go through
>> the proper RFC process.
>>
>> Please tell me what you think.
>>
>>
> I think that it makes sense in general. There are just couple of things
> that are not described in RFC.
>
> Currently it has higher priority than JSON_PARTIAL_OUTPUT_ON_ERROR. I think
> that if someone specify this constant, then the partial output is required
> and it should be preferred. In any case I think it should be in the RFC
> what the behavior is.

Per your suggestion, this has been changed so
JSON_PARTIAL_OUTPUT_ON_ERROR takes precedence. This means if you have a
wrapper function that just adds a JSON_THROW_ON_ERROR flag, you can
still use JSON_PARTIAL_OUTPUT_ON_ERROR. And the RFC mentions this.

>
> Another thing is the logic of setting global json error. Currently it just
> reset the error value so the next call to json_last_error() will return 0
> (no error) even though there was an error which is in the exception code. I
> think that the behavior should be either not resetting the global error
> value at all (the previous error should be kept) or setting the global
> error value so the json_last_error returns the same value as exception code
> or 0 if no error. Again it would be good to mention the behavior in the RFC
> so we prevent a new discussion in the PR... :)

I'm not sure what to do here. I want code to use one error mechanism or
the other, not both, so I don't want JSON_THROW_ON_ERROR to also set the
global error code. I decided to reset it to no error because there's no
previous error that needs handling by error-checking code, the exception
takes care of that. But I can see where you're coming from with the idea
of not touching it at all.

Thanks.

>
> Cheers
>

--
Andrea Faulds
https://ajf.me/