[PHP-DEV][RFC][VOTE] Throwable error code's type generalization

Greeting fellow elePHPants and happy new year.

I've just started the vote for the RFC in subject. You can find it here:

https://wiki.php.net/rfc/throwable-code-generalization

Hope it's all fine because this is my first RFC... :P

Voting will end 13 Jan at 5 PM (UTC).

Thank you.

Hi Wes,



On 6 Jan 2017 18:03, "Wes" <[email protected]> wrote:

Greeting fellow elePHPants and happy new year.

I've just started the vote for the RFC in subject. You can find it here:

https://wiki.php.net/rfc/throwable-code-generalization

Hope it's all fine because this is my first RFC... :P

Voting will end 13 Jan at 5 PM (UTC).

Thank you.


As previously discussed, this is a BC break that affects any consumers of
`Throwable` or `Exception` using `getCode()`.

The fix for PDO is not "allow any garbage in the exception code", but "fix
PDO", reducing the BC break surface to a minimum.

Adding an explicit `int` return type check is sufficient and would get
immediate feedback from people trying new RCs.

Changing an interface (!!!!!!!) by breaking covariance is not a solution.

If you need to add more context to an exception, consider adding a new
interface that just defines an accessor for that new context information.

Greets,

Marco Pivetta

Hi Marco \o,

linking to the discussion thread http://externals.io/thread/573 because I
don't have much more to add.

I think the throwable's code is almost never used regardless, but this
could give users more opportunities to do something useful with it as
Niklas stated in previous thread.

Changing PDO errors to be int is an option, too, but I don't see that
happening, not even in a major version. It would be a huge non-bc change
for very very little improvement...

This instead could open up to new uses for the property, that again is
barely used these days, while also fixing the PDO inconsistency.

On 6 Jan 2017 18:27, "Wes" <[email protected]> wrote:

Hi Marco \o,

linking to the discussion thread http://externals.io/thread/573 because I
don't have much more to add.

I think the throwable's code is almost never used regardless, but this
could give users more opportunities to do something useful with it as
Niklas stated in previous thread.

Changing PDO errors to be int is an option, too, but I don't see that
happening, not even in a major version. It would be a huge non-bc change
for very very little improvement...

This instead could open up to new uses for the property, that again is
barely used these days, while also fixing the PDO inconsistency.


`Exception#getCode()` is mostly used when re-throwing exceptions as
different exception types, and the API naming makes for very hard searches
for usages on Github too.

In doctrine, we use it to exactly to decide what to throw (specific
exception type).

If you are saying that `getCode` is mostly unused, consider proposing a
deprecation RFC instead, and provide an API specific to your use case as a
replacement.

Deprecating it is also a lot of effort for very little improvement. People
would be forced to fix all their constructors calls just to skip the
parameter. Personally I would be very annoyed by such a pointless change.

Plus those that actually do use the code will be forced eventually to
reimplement the code thing themselves, they'll end up having a base class
for this, and other people will do the same just to maintain compatibility
with older PHPs, and then even more people will do the same, human
sacrifice, dogs and cats living together... mass hysteria.

I think this is absolutely unnecessary, as you can simply not using
"getCode()" if you don't need it, and because adding a "0" in the
parent::__construct(..., 0, ...) is not too much work. With this change we
keep old code covered, *and* we enable people to use the field in more ways
if they desire to.

\o

>
> linking to the discussion thread http://externals.io/thread/573 because I
> don't have much more to add.
>
> I think the throwable's code is almost never used regardless, but this
> could give users more opportunities to do something useful with it as
> Niklas stated in previous thread.
>

Hi Wes,

why did you go for `mixed` instead of `string|int`? Error codes being
objects still doesn't make sense to me.

Regards, Niklas

I don't see how objects are less important than strings. Many of us have
enum-ish kind of objects in their code, even if php doesn't support them
natively. We could consider using them as error code if this was allowed.

2017-01-08 16:20 GMT+01:00 Wes <[email protected]>:

> I don't see how objects are less important than strings. Many of us have
> enum-ish kind of objects in their code, even if php doesn't support them
> natively. We could consider using them as error code if this was allowed.
>

An object is nothing I can really stringify and use in log aggregations.

Regards, Niklas

Yes, you can.
http://php.net/manual/en/language.oop5.magic.php#object.tostring :P

Hi Wes,

2017-01-08 15:44 GMT-04:00 Wes <[email protected]>:

> Yes, you can.
> http://php.net/manual/en/language.oop5.magic.php#object.tostring :P
>

It's still not guaranteed that the returned value is "stringifiable" as it
was before. Logging code would now need is_object + method_exist checks and
so on.

I'm considering to go -1 on this change.

that is actually a good point Marcio.

2017-01-08 21:10 GMT+01:00 Marcio Almada <[email protected]>:

> Hi Wes,
>
> 2017-01-08 15:44 GMT-04:00 Wes <[email protected]>:
>
>> Yes, you can.
>> http://php.net/manual/en/language.oop5.magic.php#object.tostring :P
>>
>
> It's still not guaranteed that the returned value is "stringifiable" as it
> was before. Logging code would now need is_object + method_exist checks and
> so on.
>
> I'm considering to go -1 on this change.
>
>

vote restarted

Voting will end on Jan 27, if you are ok with that.
I have been told one week wasn't enough, then also the mailing list
problems happened.
This should be enough to give everyone the opportunity to vote.
Thank you.

Hi,

On 06.01.17 18:02, Wes wrote:
> Greeting fellow elePHPants and happy new year.
>
> I've just started the vote for the RFC in subject. You can find it here:
>
> https://wiki.php.net/rfc/throwable-code-generalization

The RFC states:
"In practice this is mostly a documentation change ... "

Does this now mean there will be a code patch for \Exception and \Error
or not?

IMHO the text is not so clear on that and no diff/patch/PR is linked.

thanks,
- Markus

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Closing this. Thanks to the voters!

Markus: sorry if i didn't answer, i didn't get the notification. I realize
now it wasn't really clear. Patch would've changed Error and Exception
constructors only, from int to "mixed". And someone else would have had to
write it, because I have no internals knowledge :)