[PSR-11] Remove ContainerException

[Matthieu Napoli]

Hi,

PSR-11, aka ContainerInterface, has been sleeping for too long. Let's get that PSR moving!

Here is a change I would like to suggest: remove the interface ContainerException.

After years of using container-interop and ContainerInterface I have not seen a use case for that exception. We initially added it to represent any exception that could happen in a container. But as far I can I see, it's never useful, in other words it could as well be "\Exception" and the result would be the same. Here is the original discussion that lead to such interface: https://github.com/container-interop/container-interop/issues/3#issuecomment-33133155

Here is the pull request to remove the interface: https://github.com/php-fig/container/pull/2

Just to be clear: I am not suggesting to remove NotFoundException, this exception is useful.

If anyone has ever seen a use case for that exception, please let me know :)

Matthieu

[Glenn Eggleton]

I believe removal of it is going to cause a lot of BC Breaks...

For example in Slim we do extend from ContainerException

[1]: https://github.com/slimphp/Slim/blob/3.x/Slim/Exception/ContainerException.php

We do actually use it.

https://gist.github.com/geggleto/86d039f6f4924b416e6fb1bd1538e269

Cheers

[Fabien Potencier]

On 8/15/16 13:13, Glenn Eggleton wrote:
> I believe removal of it is going to cause a lot of BC Breaks...

They can't have BC breaks on something that is not standardized yet.
PSR-11 is still a work in progress as far as I know.

> For example in Slim we do extend from *ContainerException*
> *
> *

> [1]: https://github.com/slimphp/Slim/blob/3.x/Slim/Exception/ContainerException.php
>
> We do actually use it.
>
> https://gist.github.com/geggleto/86d039f6f4924b416e6fb1bd1538e269
>
> Cheers
>
> On Monday, August 15, 2016 at 3:10:07 PM UTC-4, Matthieu Napoli wrote:
>
> Hi,
>
> PSR-11, aka ContainerInterface, has been sleeping for too long.
> Let's get that PSR moving!
>

> Here is a change I would like to suggest: *remove the interface
> ContainerException*.

>
> After years of using container-interop and ContainerInterface I have
> not seen a use case for that exception. We initially added it to
> represent any exception that could happen in a container. But as far
> I can I see, it's never useful, in other words it could as well be
> "\Exception" and the result would be the same. Here is the original
> discussion that lead to such
> interface: https://github.com/container-interop/container-interop/issues/3#issuecomment-33133155
> <https://github.com/container-interop/container-interop/issues/3#issuecomment-33133155>
>
> Here is the pull request to remove the
> interface: https://github.com/php-fig/container/pull/2
> <https://github.com/php-fig/container/pull/2>
>
> Just to be clear: I am not suggesting to remove NotFoundException,
> this exception is useful.
>
> If anyone has ever seen a use case for that exception, please let me
> know :)
>
> Matthieu
>

> --
> You received this message because you are subscribed to the Google
> Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to php-fig+u...@googlegroups.com
> <mailto:php-fig+u...@googlegroups.com>.
> To post to this group, send email to php...@googlegroups.com
> <mailto:php...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/php-fig/8be1eb67-6144-4e11-854c-fa3b9e501988%40googlegroups.com
> <https://groups.google.com/d/msgid/php-fig/8be1eb67-6144-4e11-854c-fa3b9e501988%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[Matthieu Napoli]

Fabien,

> I believe removal of it is going to cause a lot of BC Breaks...

They can't have BC breaks on something that is not standardized yet.
PSR-11 is still a work in progress as far as I know.

Indeed PSR-11 is still in progress. I think Glenn was referring to the container-interop standard.

Glenn,

FYI container-interop would not be affected by any change in the PSR, those are separate. As a matter of fact container-interop will probably be deprecated if PSR-11 is approved, but the package will continue to work anyway. Moving from container-interop to PSR-11 however will require some adjustements (maybe some breaks, but I think there are workarounds) because the package will be different, the namespace too, etc. So it's not really an issue with the interface being discussed here.

In Slim you indeed use the interface but you only implement it, like all other compatible containers. However my point is that this interface is never *caught*, so it's not really useful, it serves no real purpose AFAIK.

Matthieu

[Paul Jones]

> On Aug 15, 2016, at 14:10, Matthieu Napoli <matt...@mnapoli.fr> wrote:
>
> Hi,
>
> PSR-11, aka ContainerInterface, has been sleeping for too long. Let's get that PSR moving!

Woohoo!

> Here is a change I would like to suggest: remove the interface ContainerException.

...

> After years of using container-interop and ContainerInterface I have not seen a use case for that exception. We initially added it to represent any exception that could happen in a container.

On principle alone, I usually like to see a package-specific base exception class, so that you can catch any/every exception from a particular package. (Maybe that's more an unnecessary consistency on my part.)


--

Paul M. Jones
http://paul-m-jones.com

[David Négrier]

PSR-11, aka ContainerInterface, has been sleeping for too long. Let's get that PSR moving!

Yeah! Let's do this!

Here is some data from Packanalyst to think on more concrete data: http://packanalyst.com/class?q=Interop%5CContainer%5CException%5CContainerException

The "Type hierarchy" section contains all the implemented exceptions. As you can see, there are plenty of classes implementing this exception.

At the very bottom of the page, there is the "Interface usage" section (that refers to all the files that are actually "using" this exception. There are 15 files only (and most of them are false positive... I need to work on this feature yet).

On a personal note, I have implemented this exception many times (to detect misconfigurations, circular references, etc...), but I have never found a valid use case where I would actually want to catch it.

I understand Paul's idea to have a package-specific base exception, but nothing prevents implementing package to have their own base exception. Since this PSR contains only 1 useful exception (NotFoundException), it might not make a lot of sense to keep an additional "base exception".

So as far as I'm concerned, I see no objections to removing the ContainerException from PSR-11.

On a side note, regarding migration from container-interop to PSR-11, we will probably deprecate container-interop in favor of PSR-11 when it's ready. It the PSR-11 Container interface stays the same, it will be "compatible" with container-interop's ContainerInterface. We could therefore publish a version 1.1 of container-interop's ContainerInterface that simply extends PSR-11's ContainerInterface. That would "automagically" migrate all container-interop projects to PSR-11. This is of course assuming that there is no BC breaks between container-interop and PSR-11.

++

David.

[Chuck Burgess]

PEAR most definitely wants to keep the package-level base exception, as it is a convention we always expect to be available.  We have *required* this for years, originally as a class in PEAR1 standards, until we came up with the idea of it instead being an interface while working on the PEAR2 standards.  We were quite happy with its demonstrated benefits, which proved even better as an interface than a class [1], and were tickled to see the use of it spread into the community after we published it.

We would prefer to *not* lose the base exception interface requirement.
CRB

[1] -- https://wiki.php.net/pear/rfc/pear2_exception_policy

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.

To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscribe@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/325BF45B-4638-43AA-9D6F-59BADB38C65E%40gmail.com.

[Matthieu Napoli]

Hi Chuck,

Are you using ContainerInterface or do you plan to use it in PEAR? If so, in which scenario would that exception be useful?

The goal is to collect such scenarios to see if that specific interface serves any purpose.

You are welcome of course to create a base exception class (or interface) in PEAR's container package to satisfy your internal conventions.

Matthieu

You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/_vdn5nLuPBI/unsubscribe.

To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CANsgjnuZ%2B-MW1Avdu67RCqKQXkEA2YUSs%2BMrJqerRK3gXn6q%2BA%40mail.gmail.com.

[David Négrier]

Hey Chuck,

PEAR FAQ says:

> By implementing the base package exception interface, all exceptions for a given pear package can be caught with catch (\PEAR2\PackageName\Exception $e).

But in PSR-11, there is only one (useful) exception interface: NotFoundException.

Aknowledging the fact there is only one (and will be only one) useful exception interface for this package, does it still make sense to keep the base exception interface?

David.

Le mer. 17 août 2016 à 12:57, Chuck Burgess <demon...@gmail.com> a écrit :

PEAR most definitely wants to keep the package-level base exception, as it is a convention we always expect to be available.  We have *required* this for years, originally as a class in PEAR1 standards, until we came up with the idea of it instead being an interface while working on the PEAR2 standards.  We were quite happy with its demonstrated benefits, which proved even better as an interface than a class [1], and were tickled to see the use of it spread into the community after we published it.

We would prefer to *not* lose the base exception interface requirement.
CRB

[1] -- https://wiki.php.net/pear/rfc/pear2_exception_policy

On Aug 15, 2016 7:35 PM, "Paul Jones" <pmjo...@gmail.com> wrote:

> On Aug 15, 2016, at 14:10, Matthieu Napoli <matt...@mnapoli.fr> wrote:
>
> Hi,

>
> PSR-11, aka ContainerInterface, has been sleeping for too long. Let's get that PSR moving!

Woohoo!


> Here is a change I would like to suggest: remove the interface ContainerException.
...
> After years of using container-interop and ContainerInterface I have not seen a use case for that exception. We initially added it to represent any exception that could happen in a container.

On principle alone, I usually like to see a package-specific base exception class, so that you can catch any/every exception from a particular package. (Maybe that's more an unnecessary consistency on my part.)


--

Paul M. Jones
http://paul-m-jones.com



--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.

To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/325BF45B-4638-43AA-9D6F-59BADB38C65E%40gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/_vdn5nLuPBI/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CANsgjnuZ%2B-MW1Avdu67RCqKQXkEA2YUSs%2BMrJqerRK3gXn6q%2BA%40mail.gmail.com.

[David Négrier]

Adjugé! Je vais faire de mon mieux pour suivre les discussions.

[Chuck Burgess]

Hey there Matthieu,

As a consumer of a PSR-implementing library, not only would I only type-hint against the PSR interfaces rather than the library's elements, I would also only type-hint against the PSR exceptions for catches.  Unless my consumer code's catch is specifically able to deal with one specific PSR exception (e.g. NotFoundException), then I would generally only catch against the library's base exception, thus able to generically contain the scenario of the library itself throwing whatever it might throw.  In this case, I understand the feeling that having only one (specific) exception can make a base exception seem unnecessary.  However, I think keeping it not only allows for the convention/expectation that consumers may already have... it also future-proofs my consumer code for when this PSR is BC-superceded and adds more exceptions.

A less prevalent case might be the implementing library having its own larger set of exceptions, where it won't necessarily make sense for them all to extend/implement the one NotFoundException.  The worse alternative there would be for the library have and throw its own... now my consumer code has to know about them rather than staying only PSR-aware.  If the base exception is available, the library can extend/implement from it, and thus all library-added exceptions can be caught by catching the PSR base exception alone.
CRB

To unsubscribe from this group and all its topics, send an email to php-fig+unsubscribe@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CANsgjnuZ%2B-MW1Avdu67RCqKQXkEA2YUSs%2BMrJqerRK3gXn6q%2BA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscribe@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/1471433444.1345100.697863737.35716C31%40webmail.messagingengine.com.

[Matthew Weier O'Phinney]

On Wed, Aug 17, 2016 at 3:07 AM, David Négrier <david....@gmail.com> wrote:
> I understand Paul's idea to have a package-specific base exception, but
> nothing prevents implementing package to have their own base exception.
> Since this PSR contains only 1 useful exception (NotFoundException), it
> might not make a lot of sense to keep an additional "base exception".

In zend-servicemanager, we've setup our package ExceptionInterface to
extend ContainerException, and have several additional exceptions that
we trigger other than "not found" conditions:

- ContainerModificationsNotAllowed. By default, the SM is setup to not
allow changes to services already defined, and this exception is
raised when attempts are made to do so.
- CyclicAlias. We have cyclic detection algorithms, and this exception
is triggered when one is detected. This is actually *very* useful when
putting together an application made of many libraries.
- InvalidService. In addition to a normal application-level DI
container, we also provide context-specific locators for plugins.
These provide algorithms internally to ensure that the plugin instance
is valid for the context (usually this means it implements a given
interface); we trigger this exception when it is not.
- ServiceNotCreated. This is different than "not found"; usually it
means that a dependency or pre-condition was not met, and the
container could not create the service.

While I rarely see code that *catches* container exceptions, having
the granularity is nice, and I could potentially see writing plugins
for systems such as Zend's Z-Ray, Blackfire, etc., that could provide
reporting based on caught container exceptions:

switch (true) {
case ($exception instanceof \Interop\Container\ContainerException):
// report container exception details,
// which might include introspection for available
// service definitions
break;
case ($exception instanceof \Psr\Log\InvalidArgumentException):
// report psr-3 exceptions
break;
case ($exception instanceof \Zend\Mvc\Exception\ExceptionInterface):
// report zend-mvc exceptions
// etc.
break;
}


As such, I'm a proponent of keeping the base exception interface.

<snip>

> On a side note, regarding migration from container-interop to PSR-11, we
> will probably deprecate container-interop in favor of PSR-11 when it's
> ready. It the PSR-11 Container interface stays the same, it will be
> "compatible" with container-interop's ContainerInterface. We could therefore
> publish a version 1.1 of container-interop's ContainerInterface that simply
> extends PSR-11's ContainerInterface. That would "automagically" migrate all
> container-interop projects to PSR-11. This is of course assuming that there
> is no BC breaks between container-interop and PSR-11.

We did this with zend-servicemanager, too — our own
ServiceLocatorInterface was compatible with container-interop, so we
modified ours to extend the container-interop interface; users were
then immediately able to use our package anywhere container-interop
was expected, with no BC breaks.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Larry Garfield]

This is the logic we had for PSR-6, as well.  There are all sorts of unexpected exceptions that an underlying implementation might throw, perhaps even indirectly via another backend.  Those should be made consistent between implementations so that I don't have to worry about those differences.

A spec is as much about the failure cases as the success cases.

Recall also that PHP has the ability to nest exceptions, so it's entirely reasonable to expect an underlying implementation to do something like:

catch (\Exception $e) {
  throw new ContainerException($e->getMessage(), $e->getCode(), $e);
}

Which would give us a catchable ContainerException, with a human-friendly message/code, and access to the underlying implementation-specific exception as well if appropriate for debugging.

PEAR got this one right, IMO, and it's worth emulating.  I'd favor keeping the common exception, although it is better as an interface than a base class.

--Larry Garfield

[Daniel Plainview]

> There are all sorts of unexpected exceptions that an underlying implementation might throw

Why you do expect (catch) unexpected exceptions? What does catch block contain? What's the difference with catch (\Exception)?

[Matthieu Napoli]

Hi all, thanks for participating in this discussion.

While David and I feel the same, it seems we are alone. We do not see real usage (not implementations or "what ifs" but actual usage) of that interface today (see https://github.com/php-fig/container/pull/2).

However its not worth blocking PSR-11 any longer because of such a detail, so let's go with the majority and move forward!

Matthieu

[Daniel Plainview]

This conversion (like some others in this group) looks like the following:

— Why do we need X? 

— Because we can do Y with it!

— Are you sure that Y is a good thing?

— *no answer*.

— OK, let's keep X.

Just for clarity, arguments for this exception are:

* "PEAR most definitely wants to keep the package-level base exception, as it is a convention we always expect to be available" -- Because PEAR does it.

* "I would also only type-hint against the PSR exceptions for catches." --  Because we can do Y. 

* "having the granularity is nice, and I could potentially see writing plugins for systems such as Zend's Z-Ray, Blackfire, etc." -- Interesting point. However, I think it has high cost (boilerplate code, noise in shiny interfaces, etc.).

* "There are all sorts of unexpected exceptions that an underlying implementation might throw .. Those should be made consistent between implementations so that I don't have to worry about those differences." -- No difference with catch (\Exception)

* "Which would give us a catchable ContainerException, with a human-friendly message/code, and access to the underlying implementation-specific exception as well if appropriate for debugging." -- If the point is to have human-friendly message, you can rethrow it with any another exception class, i.e. no difference with throw new \RuntimeException('User-friendly message', $e).

 

I have strong feeling that all these exception madness comes from the fact that PHP has no concept of checked/unchecked exceptions at language like Java, C# and others.

It leads to misunderstandings: do we break BC if we would throw exception like \InvalidArgumentException? Should we declare ContainerException in the interface? Etc.

If we took into account checked/unchecked separation, we would find out that most of noisy and technical exceptions are not supposed to be part of public API (interface).

We wouldn't discuss a lot about "Does \InvalidArgumentException break BC?"; we wouldn't see many technical notes like this one.

Consider NullPointerException from Java.

Does it make sense to rethrow NullPointerException as ContainerException?

Does it OK for you that you'd have bigger trace because of 2 exceptions (ContainerException + previous)? It makes debugging harder I think, not easier like someone said above. 

Any method can throw NullPointerException. Code must deal with it. It makes no sense to convert it to ContainerException.

ContainerException doesn't allow you to understand what exactly is wrong with container.

If you just want to prevent program crash because of it, you'd catch just \Exception or even \Throwable, no point to catch ContainerException.

ContainerException is unnecessary noise in your shiny interface. Like any other noise it moves developer away from the domain (DI containers in this case), it makes him think about unnecessary technical details. It doesn't help, seriously. 

[Krzysiek Piasecki]

W dniu poniedziałek, 15 sierpnia 2016 21:10:07 UTC+2 użytkownik Matthieu Napoli napisał:

Hi,

PSR-11, aka ContainerInterface, has been sleeping for too long. Let's get that PSR moving!

Here is a change I would like to suggest: remove the interface ContainerException.

Hello,

IMO both interfaces are questionable.

1. ContainerException

- One interface to represents a runtime and logic exceptions so at the end we should always base on the implementation. - No clear context. - Enforces exception implementation.

2. NotFoundException

- Not found what? Container was not found? - No clear context.

The whole idea to use interfaces with exceptions to mark 'something' is totally impractical. If we think that something is very important we should provide the exception classes derived from standard exception hierarchy, just like we do providing PSR interfaces, because classes are just interfaces.

-- 

KP

 

[David Négrier]

Hey Krzysiek!

1. ContainerException

- One interface to represents a runtime and logic exceptions so at the end we should always base on the implementation. - No clear context. - Enforces exception implementation.

I do agree that the fact that the ContainerExceptionInterface can be implemented by both runtime and logic exception makes it very impractical to use. I think everybody here agrees with this point of view (even the proponents for keeping this interface). I you read carefully Matthew's mail, his point is that the exception could be used for some reporting tools (like Zend's Z-Ray or Blackfire). He would like to keep the exception in place for these use cases.

 

2. NotFoundException

- Not found what? Container was not found? - No clear context.

The current PSR draft is quite clear:

A call to the get method with a non-existing id MUST throw a Psr\Container\Exception\NotFoundExceptionInterface.

I'm not sure I understand your comment. Do you mean we should rename it to something more specific, like EntryNotFoundExceptionInterface?

 

The whole idea to use interfaces with exceptions to mark 'something' is totally impractical. If we think that something is very important we should provide the exception classes derived from standard exception hierarchy, just like we do providing PSR interfaces, because classes are just interfaces.

I strongly disagree with you here. Each framework has its own hierarchy of exceptions. Requiring an existing framework to throw an exception provided by this PSR would likely cause breaking changes in the framework. Have a look at Symfony for instance. The container has already a "has" and a "get" method. Implementing PSR-11 would be a breeze (if they want to). If however we require them to use a ContainerException "class" (instead of an interface), that adoption could not be done before Symfony 4 (because Symfony currently has its own NotFoundException and it cannot change it without breaking the API).


Best regards,
David.