Hi Dan,
Sorry, I think I should directly address your points.
Dan Ackroyd wrote:
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.
That difference came about, IIRC, because someone objected to the
behaviour in, I think, a pull request comment. I think they were right,
because…
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.
The problem with that code is it mixes two intentionally separate
error-handling mechanisms.
If you want to use the old last_error system:
$bar = json_encode('Hello world!');
if (json_last_error() !== JSON_ERROR_NONE) {
// Handle error case
}
If you want to use the new exception system:
try {
$bar = json_encode('Hello world!', JSON_THROW_ON_ERROR);
} catch (JsonException $err) {
// Handle error case
}
These examples are internally consistent. The first (implicitly)
requests the old error handling approach, then checks for an error the
old way. The second explicitly requests the new error handling approach,
then checks for an error the new way.
But your new example is not internally consistent, it asks for the new
approach then tries to handle it the old way. It's true, it will break
if the old-system global error state is dirty. But it will also break if
there is an actual error:
$bar = json_encode($unencodable, JSON_THROW_ON_ERROR); // throws
JsonException, unhandled
The following line checking json_last_error() would never even be executed.
Let's imagine we were to make json_last_error() still be set if
JSON_THROW_ON_ERROR was set. It would then be possible to write this:
try {
$bar = json_encode('Hello world!', JSON_THROW_ON_ERROR);
} catch (JsonException $err) {
if (json_last_error() !== JSON_ERROR_NONE) {
// Handle error
}
}
But this is redundant, because when we've caught a JsonException we
already know there was an error. Likewise if we just use
json_last_error() to find out what type of error, this is also
redundant, because $err already contains that information.
It would also, in that hypothetical scenario, be possible to write this:
try {
$bar = json_encode('Hello world!', JSON_THROW_ON_ERROR);
} catch (JsonException $err) { }
if (json_last_error() !== JSON_ERROR_NONE) {
// Handle error
}
This… would work, but it defeats the purpose of using
JSON_THROW_ON_ERROR at all.
I agree it might be surprising to have json_last_error() not be touched
when you use JSON_THROW_ON_ERROR, but I assign more value to keeping the
error mechanisms cleanly separated than to allowing strangely-written
self-defeating code to not crash.
At first I assumed you were making an argument about compatibility
between PHP versions, where on an older version the JSON_THROW_ON_ERROR
constant wouldn't do anything. But because the constant wouldn't exist,
you couldn't use it without causing some error, so you'd really need to
write:
if (constant('JSON_THROW_ON_ERROR') !== NULL) {
$bar = json_encode($foo, JSON_THROW_ON_ERROR);
} else {
$bar = json_encode($foo);
if (json_last_error() !== JSON_ERROR_NONE) {
throw new Exception(…);
}
}
Thanks,
Andrea