Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105653
MIME-Version: 1.0
Date: Thu, 9 May 2019 17:06:34 +0100
Message-ID: <CA+kxMuS6gnPZqd-e5UV_zmE3q9ZpXS+UBSGNvF7hiagY15LFJw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: JSON_THROW_ON_ERROR implementation detail
From: Danack@basereality.com (Dan Ackroyd)

Hi,

Apparently there is an implementation detail in JSON_THROW_ON_ERROR
that differs in the RFC text, from the discussion on list
http://news.php.net/php.internals/100569:

> 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.

RFC text:

> When passed this flag, the error behaviour of these functions is changed.
> The global  error state is left untouched, and if an error occurs that would
> otherwise set it, these functions instead throw a JsonException with the
> message and code set to whatever json_last_error() and json_last_error_msg()
> would otherwise be respectively.

The implementation is highly surprising and can lead to bugs.

Imagine this code exists in a code base.

---------------------

$foo = json_decode('[bar');
// many lines of code.
// many lines of code.
// many lines of code.

$bar = json_encode('Hello world!');
if (json_last_error() !== JSON_ERROR_NONE) {
    throw new Exception("encoding went wrong!");
}

echo "All is fine";
// output is "All is fine"
---------------------


And to start moving to have json_encode throw on error, they start
using the flag on the json_encode

----------------------

$foo = json_decode('[bar');
// many lines of code.
// many lines of code.
// many lines of code.

$bar = json_encode('Hello world!', JSON_THROW_ON_ERROR);
if (json_last_error() !== JSON_ERROR_NONE) {
    throw new Exception("encoding went wrong!");
}

echo "All is fine";

-------------------

The result of this is that the user throw exception will be thrown
even though the encode worked correctly. https://3v4l.org/JJD5g

This is highly surprising*, and doesn't seem the right thing to be doing.

Does anyone have any objection to changing this, so that
json_last_error() + json_last_error_msg() always refer to the most
recent json_encode()/json_decode() function, as per the discussion. Or
does this need an RFC?

I've logged a bug for this regardless - https://bugs.php.net/bug.php?id=77997

cheers
Dan
Ack

* https://en.wikipedia.org/wiki/Principle_of_least_astonishment