Everything

php

[RFC Discussion] Catching multiple exception types

php.internals

Pierrick Charron

10 years ago

Hi internals,

Bronisław Białek and I would like to start a discussion about allowing
multiple exception types to be caught in a single catch statement.

https://wiki.php.net/rfc/multiple-catch

A working implementation and tests are available in the RFC.

We are waiting for your constructive feedback and thoughts.

Thanks
Pierrick

Sean DuBois

10 years ago

On Tue, Mar 08, 2016 at 04:42:29PM -0500, Pierrick Charron wrote:

> Hi internals,
>
> Bronisław Białek and I would like to start a discussion about allowing
> multiple exception types to be caught in a single catch statement.
>
> https://wiki.php.net/rfc/multiple-catch
>
> A working implementation and tests are available in the RFC.
>
> We are waiting for your constructive feedback and thoughts.
>
> Thanks
> Pierrick

Hi!

It may be an issue that this is close in design to [0] union types.
This RFC would be *great* for code quality, just might be a subset of
union types.

thanks

[0] https://wiki.php.net/rfc/union_types

Yasuo Ohgaki

10 years ago

Hi all,

On Wed, Mar 9, 2016 at 6:50 AM, Sean DuBois <sean@siobud.com> wrote:

> It may be an issue that this is close in design to [0] union types.
> This RFC would be *great* for code quality, just might be a subset of
> union types.
>
> thanks
>
> [0] https://wiki.php.net/rfc/union_types

Nice and handy!
If we are going to support this kind of code, union types seem better way.
However, I have mixed feeling..

Current PHP has issues like string literal/string type/__toStringed
object are treated differently. For instance,
http://www.phpwtf.org/instanceof-smart

I would like to sort out and fix these kind of issues first. i.e. Let
array type Traversable

Just my .02
Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Pierrick Charron

10 years ago

Hi Sean and Yasuo

I agree that they are somehow related, but I also think (and I might be
wrong) that the behaviour of the Multi-Catch is obvious and I don't see how
this syntax/feature could be interpreted otherwise. The multi-catch don't
have any open-issue that the Union-Types have like weak types since it only
deals with Exceptions. Also, we still don't know if we'll one day have the
Union-Types since there are those open issues and it would be sad to wait
something that could never append. It's IMO pretty safe going forward and
if required in the future change the implementation when needed (Behaviour
will stay the same, unless you see other behaviour that this syntax could
have in this same "catch" context).

Pierrick

On 8 March 2016 at 16:50, Sean DuBois <sean@siobud.com> wrote:

Björn Larsson

10 years ago

Den 2016-03-08 kl. 22:42, skrev Pierrick Charron:

> Hi internals,
>
> Bronisław Białek and I would like to start a discussion about allowing
> multiple exception types to be caught in a single catch statement.
>
> https://wiki.php.net/rfc/multiple-catch
>
> A working implementation and tests are available in the RFC.
>
> We are waiting for your constructive feedback and thoughts.
>
> Thanks
> Pierrick
>

Nice RFC! Think it would be good if you had an example in the
RFC showing the applicability of catching two php exceptions.
Especially given the new exception hierarchy in PHP 7. I'm also
pondering if the main target for this is custom exceptions or
the built-in ones or both?

Regards //Björn Larsson

PS

Pierrick Charron

10 years ago

Hi Björn,

The only time I had to do this with core PHP exceptions is to make the code
compatible for both PHP5 and PHP7:

try {
} catch(\Exceptions $e) {
} catch(\Throwable $e) {
}

But it will of course not be applicable since this feature is targeting
PHP7.1. Other than that the PHP core exception hierarchy is well enough for
MY needs. But if someone already had to do this fill free to provide your
use case as an example.

My main target is custom exceptions (even if the logic is applicable on
everything Throwable). A custom exception use case would be some method
that throw thwo different kind of exceptions like for example the doctrine
AbstractQuery::getSingleResult (NoResultException,
NonUniqueResultException) that you could want to handle the same way.

An other really easy example would be simple code like this one that I
found in symfony (not really a big deal but still)

} catch (AccessException $e) {
return false;
} catch (UnexpectedTypeException $e) {
return false;
}

And other piece of code using multiple libraries.


On 8 March 2016 at 18:06, Björn Larsson <bjorn.x.larsson@telia.com> wrote:

Bronisław Białek

10 years ago

Hi :)

I think it is feature that is useful time to time, not very often.
One example:

class PackingFailed extends \Exception implements PackagingException {}

// this exception could originate from package with assertions/exceptions,
// so I cannot use common interface with above
class EmptyName extends \Exception implements ValidationException {}

class Packer
{
/** @throws PackingFailed */
public function pack(PackTemplate $packTemplate) : Pack
{
try {
return $this->save($packTemplate);
} catch (IOException $e) {
throw new PackingFailed('', 0, $e);
}
}
}

class PackTemplate
{
/** @throws EmptyName */
public function __construct(string $name)
{
if ($name === '') {
throw new EmptyName();
}
}
}

/** @throws SomeException */
function packeForMe(string $name) : Pack
{
try {
return (new Packer())->pack(new PackTemplate($name));
} catch (PackingFailed | ValidationException $e) {
throw new SomeException($e); // or return null in other cases
}
}


2016-03-09 2:47 GMT+01:00 Pierrick Charron <pierrick@adoy.net>:

> Hi Björn,
>
> The only time I had to do this with core PHP exceptions is to make the
> code compatible for both PHP5 and PHP7:
>
> try {
> } catch(\Exceptions $e) {
> } catch(\Throwable $e) {
> }
>
> But it will of course not be applicable since this feature is targeting
> PHP7.1. Other than that the PHP core exception hierarchy is well enough for
> MY needs. But if someone already had to do this fill free to provide your
> use case as an example.
>
> My main target is custom exceptions (even if the logic is applicable on
> everything Throwable). A custom exception use case would be some method
> that throw thwo different kind of exceptions like for example the doctrine
> AbstractQuery::getSingleResult (NoResultException,
> NonUniqueResultException) that you could want to handle the same way.
>
> An other really easy example would be simple code like this one that I
> found in symfony (not really a big deal but still)
>
> } catch (AccessException $e) {
>     return false;
> } catch (UnexpectedTypeException $e) {
> return false;
> }
>
> And other piece of code using multiple libraries.
>
>
> On 8 March 2016 at 18:06, Björn Larsson <bjorn.x.larsson@telia.com> wrote:
>
>> Den 2016-03-08 kl. 22:42, skrev Pierrick Charron:
>>
>>> Hi internals,
>>>
>>> Bronisław Białek and I would like to start a discussion about allowing
>>> multiple exception types to be caught in a single catch statement.
>>>
>>> https://wiki.php.net/rfc/multiple-catch
>>>
>>> A working implementation and tests are available in the RFC.
>>>
>>> We are waiting for your constructive feedback and thoughts.
>>>
>>> Thanks
>>> Pierrick
>>>
>>> Nice RFC! Think it would be good if you had an example in the
>> RFC showing the applicability of catching two php exceptions.
>> Especially given the new exception hierarchy in PHP 7. I'm also
>> pondering if the main target for this is custom exceptions or
>> the built-in ones or both?
>>
>> Regards //Björn Larsson
>>
>> PS
>>
>
>

--
Pozdrawiam,
Bronisław Białek.

Björn Larsson

10 years ago

Hi, Thanks for clarifying. Think the RFC would benefit from having
some of these examples in it. I also had in mind that it's handy for
catching many exceptions when logging stuff and re-throwing like:

}  catch (FirstException | SecondException ex) {
logger.error(ex);
throw ex;
}

Regards //Björn

PS Sorry for top-posting...

Den 2016-03-09 kl. 08:05, skrev Bronisław Białek:

Pierrick Charron

10 years ago

Thanks for your feedback !

Any other thoughts from anyone else on this ?

On 10 March 2016 at 02:31, Björn Larsson <bjorn.x.larsson@telia.com> wrote:

Derick Rethans

10 years ago

Hi!

On Tue, 8 Mar 2016, Pierrick Charron wrote:

> Bronisław Białek and I would like to start a discussion about allowing
> multiple exception types to be caught in a single catch statement.
>
> https://wiki.php.net/rfc/multiple-catch

Would it not be better, to have your Exceptions extend a common base
class in this case? Or perhaps, which I think is even better, to have
your Exception classes implement a common interface that you can catch
against. That I believe should already work.

What you are asking PHP users to do with a multiple catch like this, is
to test in each catch's statement block to test for each class again.
Neither the inheritence or implements options from the previous
paragraph have that issue.

cheers,
Derick

Niklas Keller

10 years ago

2016-03-09 12:52 GMT+01:00 Derick Rethans <derick@php.net>:

> Hi!
>
> On Tue, 8 Mar 2016, Pierrick Charron wrote:
>
> > Bronisław Białek and I would like to start a discussion about allowing
> > multiple exception types to be caught in a single catch statement.
> >
> > https://wiki.php.net/rfc/multiple-catch
>
> Would it not be better, to have your Exceptions extend a common base
> class in this case? Or perhaps, which I think is even better, to have
> your Exception classes implement a common interface that you can catch
> against. That I believe should already work.
>

This assumes they're my exceptions and I can change the code. Doesn't work
with libraries having not that good exception structures or even leaking
their dependency's exceptions.

> What you are asking PHP users to do with a multiple catch like this, is
> to test in each catch's statement block to test for each class again.
>

No, usually it's enough that they're all extending Exception / Throwable.
You just can't catch Exception / Throwable, because that would catch all
exceptions. If you have a multi-catch, it doesn't matter which exception is
thrown and it's never checked, otherwise you'd use two blocks directly.

Regards, Niklas

Pierrick Charron

10 years ago

Hi Derick

I agree that most of the time the best solution is to implement a clean
exception hierarchy but as stated in the RFC :

"A solution to fix this problem on the user level would be to implement a
common interface for ExceptionType1 and ExceptionType2 and catch it.
However, this is only possible when you control the exception hierarchy in
your own code, but not possible when you don't control the code."

On 9 March 2016 at 06:52, Derick Rethans <derick@php.net> wrote:

Marco Pivetta

10 years ago

On 9 March 2016 at 14:03, Pierrick Charron <pierrick@adoy.net> wrote:

> Hi Derick
>
> I agree that most of the time the best solution is to implement a clean
> exception hierarchy but as stated in the RFC :
>
> "A solution to fix this problem on the user level would be to implement a
> common interface for ExceptionType1 and ExceptionType2 and catch it.
> However, this is only possible when you control the exception hierarchy in
> your own code, but not possible when you don't control the code."
>

I understand the use-case, but I don't see it as a widespread scenario. In
most cases, I've been doing something like following:

public function stuff()
{
try {
$this->willFail();
} catch (FirstException $e) {
$this->handleFailure($e);
} catch (SecondException $e) {
$this->handleFailure($e);
} catch (ThirdException $e) {
$this->handleFailure($e);
}
}

Even then, this is a rare eventuality, and as pointed out above, usually
fixed when wrapping exceptions correctly, if you have control over the
exception types).
Seems way below the 80/20 use-case to me, and introduces syntax changes as
well.

Cheers,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Pierrick Charron

10 years ago

On 9 March 2016 at 08:08, Marco Pivetta <ocramius@gmail.com> wrote:

> On 9 March 2016 at 14:03, Pierrick Charron <pierrick@adoy.net> wrote:
>
>> Hi Derick
>>
>> I agree that most of the time the best solution is to implement a clean
>> exception hierarchy but as stated in the RFC :
>>
>> "A solution to fix this problem on the user level would be to implement a
>> common interface for ExceptionType1 and ExceptionType2 and catch it.
>> However, this is only possible when you control the exception hierarchy in
>> your own code, but not possible when you don't control the code."
>>
>
> I understand the use-case, but I don't see it as a widespread scenario. In
> most cases, I've been doing something like following:
>
>

I agree that this the RFC will not get the oscar for feature of the year,
but I think it will lead in those few use-case to more readable code.

> public function stuff()
> {
>     try {
>         $this->willFail();
>     } catch (FirstException $e) {
>         $this->handleFailure($e);
>     } catch (SecondException $e) {
>         $this->handleFailure($e);
>     } catch (ThirdException $e) {
>         $this->handleFailure($e);
>     }
> }
>
>

The thing I don't like about this approach is that I have to read the code
and double check to make sure that the catch statement call the same method.
For the amount of work that needs to be done in the Engine (see the patch
attached to the RFC) this is far more readable and it is clear that the
code to handle those 3 exceptions is the exact same one. And if the code of
handleFailure is small you can even put it at this single place.

public function stuff()
{
try {
$this->willFail();
} catch (FirstException | SecondException | ThirdException $e) {
$this->handleFailure($e);
}
}

Even then, this is a rare eventuality, and as pointed out above, usually

Arvids Godjuks

10 years ago

I'm definetly lately started to use instanceof to check the type of
exception I've got and throw further those that I don't need. This is
especially valuable for CLI applications (I have a few daemons in PHP).

So a big +1 on the proposal idea, as for the tech part -  maybe unions is a
good idea.

Arvids Godjuks.

On Wed, 9 Mar 2016, 15:24 Pierrick Charron, <pierrick@adoy.net> wrote:

Marco Pivetta

10 years ago

On 9 March 2016 at 14:24, Pierrick Charron <pierrick@adoy.net> wrote:

>
> The thing I don't like about this approach is that I have to read the code
> and double check to make sure that the catch statement call the same method.
> For the amount of work that needs to be done in the Engine (see the patch
> attached to the RFC) this is far more readable and it is clear that the
> code to handle those 3 exceptions is the exact same one. And if the code of
> handleFailure is small you can even put it at this single place.
>
> public function stuff()
> {
>     try {
>         $this->willFail();
>     } catch (FirstException | SecondException | ThirdException $e) {
>         $this->handleFailure($e);
>     }
> }
>
>

I'd still have to write 3 test cases anyway: no real amount of code-savings
there.

I still think the first solution is as readable as the one you proposed
there.
The change in the PHP engine might be trivial, but the syntax change from a
userland perspective is a mess (tooling, pretty much anything that relies
on an AST parser needs changes there).
Do not underestimate language changes as a php-core only issue.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Pierrick Charron

10 years ago

On 9 March 2016 at 08:30, Marco Pivetta <ocramius@gmail.com> wrote:

> On 9 March 2016 at 14:24, Pierrick Charron <pierrick@adoy.net> wrote:
>>
>> The thing I don't like about this approach is that I have to read the
>> code and double check to make sure that the catch statement call the same
>> method.
>> For the amount of work that needs to be done in the Engine (see the patch
>> attached to the RFC) this is far more readable and it is clear that the
>> code to handle those 3 exceptions is the exact same one. And if the code of
>> handleFailure is small you can even put it at this single place.
>>
>> public function stuff()
>> {
>>     try {
>>         $this->willFail();
>>     } catch (FirstException | SecondException | ThirdException $e) {
>>         $this->handleFailure($e);
>>     }
>> }
>>
>>
> I'd still have to write 3 test cases anyway: no real amount of
> code-savings there.
>

Agree

>
> I still think the first solution is as readable as the one you proposed
> there.
>

Agree to disagree ;-)

> The change in the PHP engine might be trivial, but the syntax change from
> a userland perspective is a mess (tooling, pretty much anything that relies
> on an AST parser needs changes there).
> Do not underestimate language changes as a php-core only issue.
>

You're right but I think those impact should not prevent us to make the
language evolve even for small changes that are just like this one
syntactic sugar. With the amount of user doing PHP code on a daily basis
those changes are IMO worth it.

Patrick ALLAERT

10 years ago

Hi,

Le mer. 9 mars 2016 à 14:08, Marco Pivetta <ocramius@gmail.com> a écrit :

> On 9 March 2016 at 14:03, Pierrick Charron <pierrick@adoy.net> wrote:
>
> > Hi Derick
> >
> > I agree that most of the time the best solution is to implement a clean
> > exception hierarchy but as stated in the RFC :
> >
> > "A solution to fix this problem on the user level would be to implement a
> > common interface for ExceptionType1 and ExceptionType2 and catch it.
> > However, this is only possible when you control the exception hierarchy
> in
> > your own code, but not possible when you don't control the code."
> >
>
> I understand the use-case, but I don't see it as a widespread scenario. In
> most cases, I've been doing something like following:
>
> public function stuff()
> {
>     try {
>         $this->willFail();
>     } catch (FirstException $e) {
>         $this->handleFailure($e);
>     } catch (SecondException $e) {
>         $this->handleFailure($e);
>     } catch (ThirdException $e) {
>         $this->handleFailure($e);
>     }
> }
>
> Even then, this is a rare eventuality, and as pointed out above, usually
> fixed when wrapping exceptions correctly, if you have control over the
> exception types).
> Seems way below the 80/20 use-case to me, and introduces syntax changes as
> well.

+1

I will add to this that Exceptions must remain... exceptional! Too much
projects relies already on tens or hundreds of business exception while
those are mostly common "error" conditions that are better handled with
dedicated API calls.

To me, it doesn't look like a reasonable change when we consider the number
of tools/IDE to adapt compared to the possible usage of it.

Cheers,
Patrick

Pierrick Charron

10 years ago

Thanks for the feedback Patrick :-) I understand your concern. I added a
section to the RFC Impact section to mention that PHP tools/IDE will
require some modifications. Feel free to add some additional details in
this section if you want. People who will vote will then be aware of the
situation and I'll of course comply to the voters decision :-)

Pierrick

On 15 March 2016 at 12:23, Patrick ALLAERT <patrickallaert@php.net> wrote: