[RFC][Discussion] Enhance JSON error messages

6 months ago by juan carlos morales — view source

unread

Hello Internals.

I'm here to discuss an improvement in error messages when dealing with
JSON in PHP.

After watching Derick Rethans speak at the UK PHP Conference:

https://youtu.be/3U0DGhzSH2U?feature=shared&t=2483

I also think it would be very useful to have better error messages.

At the moment, we get things like "Syntax Error" ... period, no clue
where the JSON is corrupt or anything else. With big JSONs is very
cumberstone to get to the line and character position where the JSON
is corrupt .... for example.

I am already working in a change that would improve error messages
while working with JSONs.

What I am very close to achieve is to have error messages like "Syntax
Error - at character position X near content Y" .... something like
that.

I already know that I can do it, actually there is a Work In Progress
Pull Request for this :

https://github.com/php/php-src/pull/14672

I repeat, is work in progress. BUT it can be done. Maybe my approach
is not the best ... but one way or another .... it can be done.

Please let me know your ideas, thoughts about this.

Please, dont kill me for the actual code, is WIP.

Thanks in advance to everyone.

Juan

6 months ago by Larry Garfield — view source

unread

Hello Internals.

I'm here to discuss an improvement in error messages when dealing with
JSON in PHP.

After watching Derick Rethans speak at the UK PHP Conference:

https://youtu.be/3U0DGhzSH2U?feature=shared&t=2483

I also think it would be very useful to have better error messages.

At the moment, we get things like "Syntax Error" ... period, no clue
where the JSON is corrupt or anything else. With big JSONs is very
cumberstone to get to the line and character position where the JSON
is corrupt .... for example.

I am already working in a change that would improve error messages
while working with JSONs.

What I am very close to achieve is to have error messages like "Syntax
Error - at character position X near content Y" .... something like
that.

I already know that I can do it, actually there is a Work In Progress
Pull Request for this :

https://github.com/php/php-src/pull/14672

I repeat, is work in progress. BUT it can be done. Maybe my approach
is not the best ... but one way or another .... it can be done.

Please let me know your ideas, thoughts about this.

Please, dont kill me for the actual code, is WIP.

Thanks in advance to everyone.

Juan

I cannot comment on the implementation, but I am always in favor of error messages that do a better job of telling you what you need to fix.

--Larry Garfield

6 months ago by Gina P. Banyard — view source

unread

I also think it would be very useful to have better error messages.

At the moment, we get things like "Syntax Error" ... period, no clue
where the JSON is corrupt or anything else. With big JSONs is very
cumberstone to get to the line and character position where the JSON
is corrupt .... for example.

I am already working in a change that would improve error messages
while working with JSONs.

What I am very close to achieve is to have error messages like "Syntax
Error - at character position X near content Y" .... something like
that.

I already know that I can do it, actually there is a Work In Progress
Pull Request for this :

https://github.com/php/php-src/pull/14672

I repeat, is work in progress. BUT it can be done. Maybe my approach
is not the best ... but one way or another .... it can be done.

Please let me know your ideas, thoughts about this.

Please, dont kill me for the actual code, is WIP.

Thanks in advance to everyone.

Juan

Hello Juan,

I'm in favour of this, but also I do not think this needs an RFC.
This is just an enhancement to our current crappy parsing errors,
and an RFC feels very much overkill for this.

Best regards,

Gina P. Banyard

6 months ago by juan carlos morales — view source

unread

El jue, 27 jun 2024 a las 1:10, Gina P. Banyard (internals@gpb.moe) escribió:

I also think it would be very useful to have better error messages.

At the moment, we get things like "Syntax Error" ... period, no clue
where the JSON is corrupt or anything else. With big JSONs is very
cumberstone to get to the line and character position where the JSON
is corrupt .... for example.

I am already working in a change that would improve error messages
while working with JSONs.

What I am very close to achieve is to have error messages like "Syntax
Error - at character position X near content Y" .... something like
that.

I already know that I can do it, actually there is a Work In Progress
Pull Request for this :

https://github.com/php/php-src/pull/14672

I repeat, is work in progress. BUT it can be done. Maybe my approach
is not the best ... but one way or another .... it can be done.

Please let me know your ideas, thoughts about this.

Please, dont kill me for the actual code, is WIP.

Thanks in advance to everyone.

Juan

Hello Juan,

I'm in favour of this, but also I do not think this needs an RFC.
This is just an enhancement to our current crappy parsing errors,
and an RFC feels very much overkill for this.

Best regards,

Gina P. Banyard

Suggested by Jakub Zelenka (the main arquitect of the JSON part in php
basically), in the PULL REQUEST is better to create a brand new
function to have detailed error messages when json encoding, decoding,
validating, fails.

So, seems a new RFC will be needed.

I am working on this at the moment. Will be ready next week or I will
try to make it for next week.

I am working

6 months ago by tim@bastelstu.be — view source

unread

Suggested by Jakub Zelenka (the main arquitect of the JSON part in php
basically), in the PULL REQUEST is better to create a brand new
function to have detailed error messages when json encoding, decoding,
validating, fails.

For the folks following along, I've commented on the PR. I think that
improving the existing messages is the right choice, not adding new
functions that users will need to remember to use.

Best regards
Tim Düsterhus

6 months ago by juan carlos morales — view source

unread

El El dom, 30 de jun. de 2024 a la(s) 09:02, Tim Düsterhus tim@bastelstu.be
escribió:

Hi

Suggested by Jakub Zelenka (the main arquitect of the JSON part in php
basically), in the PULL REQUEST is better to create a brand new
function to have detailed error messages when json encoding, decoding,
validating, fails.

For the folks following along, I've commented on the PR. I think that
improving the existing messages is the right choice, not adding new
functions that users will need to remember to use.

Best regards
Tim Düsterhus

Thanks for writting Tim.

So, what I see here in my shortexperiencie is an RFC with 3 Options

Enhance the error message we already have
keep json_last_error_msg as it is and add a new function
json_last_error_info function
both

And let the voting to decide

That is what I see so far

Is evident that everyone agrees is a helpful feature to have better error
understandind

6 months ago by tim@bastelstu.be — view source

unread

So, what I see here in my shortexperiencie is an RFC with 3 Options

That is not an option. Each RFC needs a clear primary vote that is a
"Yes/No" vote.

Enhance the error message we already have

keep json_last_error_msg as it is and add a new function
json_last_error_info function

both

Adjusting error messages to make them better is not something that
requires an RFC. We do this all the time without an RFC. Here's an
example PR: https://github.com/php/php-src/pull/14496

A new json_last_error_info() for easier programmatic access of the
important bits can still be considered, but would not need an RFC either
from my point of view. It's a simple and obvious addition. Those can be
done without an RFC, unless someone complains.

Best regards
Tim Düsterhus

6 months ago by juan carlos morales — view source

unread

El El dom, 30 de jun. de 2024 a la(s) 11:15, Tim Düsterhus tim@bastelstu.be
escribió:

Hi

So, what I see here in my shortexperiencie is an RFC with 3 Options

That is not an option. Each RFC needs a clear primary vote that is a
"Yes/No" vote.

Enhance the error message we already have

keep json_last_error_msg as it is and add a new function
json_last_error_info function

both

Adjusting error messages to make them better is not something that
requires an RFC. We do this all the time without an RFC. Here's an
example PR: https://github.com/php/php-src/pull/14496

A new json_last_error_info() for easier programmatic access of the
important bits can still be considered, but would not need an RFC either
from my point of view. It's a simple and obvious addition. Those can be
done without an RFC, unless someone complains.

Best regards
Tim Düsterhus

Ok , then, i will prepare 2 PRs and lets see what happens then.

Thanks.

I will let everybody know When ready

5 months ago by juan carlos morales — view source

unread

El El dom, 30 de jun. de 2024 a la(s) 11:52, juan carlos morales <
dev.juan.morales@gmail.com> escribió:

El El dom, 30 de jun. de 2024 a la(s) 11:15, Tim Düsterhus <
tim@bastelstu.be> escribió:

Hi

So, what I see here in my shortexperiencie is an RFC with 3 Options

That is not an option. Each RFC needs a clear primary vote that is a
"Yes/No" vote.

Enhance the error message we already have

keep json_last_error_msg as it is and add a new function
json_last_error_info function

both

Adjusting error messages to make them better is not something that
requires an RFC. We do this all the time without an RFC. Here's an
example PR: https://github.com/php/php-src/pull/14496

A new json_last_error_info() for easier programmatic access of the
important bits can still be considered, but would not need an RFC either
from my point of view. It's a simple and obvious addition. Those can be
done without an RFC, unless someone complains.

Best regards
Tim Düsterhus

Ok , then, i will prepare 2 PRs and lets see what happens then.

Thanks.

I will let everybody know When ready

Hello again. Well seems there are different opinions about how to approach
this.

Everyone agrees that having better error messages is always welcome but at
the moment are different postures about having

enhanced error message
have a new function with better data
having both

At the moment the mentioned pull request has both

But i need to understand WHAT are we going to keep. And in case an RFC is
needed then i will make it.

Please advice

Thanks