Everything

php

[RFC][Discussion] Enhance JSON error messages

php.internals

juan carlos morales

2 years ago

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

Larry Garfield

2 years ago

On Wed, Jun 26, 2024, at 2:11 PM, juan carlos morales wrote:

> 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

Gina P. Banyard

2 years ago

On Wednesday, 26 June 2024 at 15:11, juan carlos morales <dev.juan.morales@gmail.com> wrote:

> 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

juan carlos morales

2 years ago

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

>
> On Wednesday, 26 June 2024 at 15:11, juan carlos morales <dev.juan.morales@gmail.com> wrote:
>
> > 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

Tim Düsterhus

2 years ago

Hi

On 6/28/24 15:49, juan carlos morales wrote:

> 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

juan carlos morales

2 years ago

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

> Hi
>
> On 6/28/24 15:49, juan carlos morales wrote:
> > 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

1) Enhance the error message we already have

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

3) 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

Tim Düsterhus

2 years ago

Hi

On 6/30/24 15:43, juan carlos morales wrote:

> 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.

> 1) Enhance the error message we already have
>
> 2) keep json_last_error_msg as it is and add a new function
> json_last_error_info function
>
> 3) 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

juan carlos morales

2 years ago

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

> Hi
>
> On 6/30/24 15:43, juan carlos morales wrote:
> > 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.
>
> > 1) Enhance the error message we already have
> >
> > 2) keep json_last_error_msg as it is and add a new function
> > json_last_error_info function
> >
> > 3) 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

juan carlos morales

2 years ago

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
>>
>> On 6/30/24 15:43, juan carlos morales wrote:
>> > 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.
>>
>> > 1) Enhance the error message we already have
>> >
>> > 2) keep json_last_error_msg as it is and add a new function
>> > json_last_error_info function
>> >
>> > 3) 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

1) enhanced error message
2) have a new function with better data

3) 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