Re: [PHP-DEV] [RFC] [Discussion] JSON_THROW_ON_ERROR

This is only part of a thread. view whole thread
  100546
September 12, 2017 16:46 bukka@php.net (Jakub Zelenka)
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
  100569
September 13, 2017 21:34 ajf@ajf.me (Andrea Faulds)
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/