[PSR-11] Clarifying exceptions

[David Négrier]

The current draft for PSR-11 contains this text relative to exceptions:

1.2 Exceptions

Exceptions directly thrown by the container MUST implement the Psr\Container\Exception\ContainerExceptionInterface.

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

We have already had a discussion regarding the usefulness of ContainerExceptionInterface. Even if I'd like to see it gone, the majority seems to be in favor of keeping it. However, there are 2 other things I'd like to discuss:

Regarding the NotFoundExceptionInterface:

The current text says a container SHOULD throw an exception. Searching through the history of container-interop, I cannot find why we put a "should" here.
I'd like to propose replacing it with a MUST. For instance, it is never ok if a container returns null instead of throwing an exception. Therefore, the need for "MUST". Does anyone has any objection regarding this change?

Regarding the ContainerExceptionInterface:

I'd like to make the opposite move and switch from MUST to SHOULD, as in: "Exceptions directly thrown by the container SHOULD implement the ContainerExceptionInterface".
Furthermore, I'd like to emphasize the "directly" part. For instance, I understand perfectly that a cyclic dependency or a config file typo should throw an exception implementing ContainerExceptionInterface. However, if the user is using a container based on factories (like Disco or PimpleInterop), and if the factory written by the user is throwing an exception, I find it useless to wrap that exception in another exception implementing the ContainerExceptionInterface. To me, it adds noise and nothing more.

Here is a sample using Disco:

/**
 * @Configuration
 */
class MyConfiguration
{
    /**
     * @Bean
     */
    public function myDbService() : DbService
    {
        ...
        ...
        throw new MissingDbDriverException();
        ...
        ...
    }
}

When calling `$container->get('myDbService')`, I would expect the container to throw a MissingDbDriverException directly. I don't want to make it compulsory for the container to wrap the MissingDbDriverException into another container-specific exception (but I don't want to forbid it either, I think it should be implementation specific...) Do you agree with this point of view? If there is a general concensus on this, maybe I can work on a few sentences in the META doc to make this clear?

++
David.

[Matthieu Napoli]

Regarding the NotFoundExceptionInterface:

The current text says a container SHOULD throw an exception. Searching through the history of container-interop, I cannot find why we put a "should" here.
I'd like to propose replacing it with a MUST. For instance, it is never ok if a container returns null instead of throwing an exception. Therefore, the need for "MUST". Does anyone has any objection regarding this change?

+1 the current text leaves indeed room for error.

 

Regarding the ContainerExceptionInterface:

I'd like to make the opposite move and switch from MUST to SHOULD, as in: "Exceptions directly thrown by the container SHOULD implement the ContainerExceptionInterface".
Furthermore, I'd like to emphasize the "directly" part. For instance, I understand perfectly that a cyclic dependency or a config file typo should throw an exception implementing ContainerExceptionInterface. However, if the user is using a container based on factories (like Disco or PimpleInterop), and if the factory written by the user is throwing an exception, I find it useless to wrap that exception in another exception implementing the ContainerExceptionInterface. To me, it adds noise and nothing more.

I have nothing against that, you are welcome to change it.

Matthieu

[Larry Garfield]

On 08/22/2016 07:51 AM, David Négrier wrote:

The current draft for PSR-11 contains this text relative to exceptions:

1.2 Exceptions

Exceptions directly thrown by the container MUST implement the Psr\Container\Exception\ContainerExceptionInterface.

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

We have already had a discussion regarding the usefulness of ContainerExceptionInterface. Even if I'd like to see it gone, the majority seems to be in favor of keeping it. However, there are 2 other things I'd like to discuss:

Regarding the NotFoundExceptionInterface:

The current text says a container SHOULD throw an exception. Searching through the history of container-interop, I cannot find why we put a "should" here.
I'd like to propose replacing it with a MUST. For instance, it is never ok if a container returns null instead of throwing an exception. Therefore, the need for "MUST". Does anyone has any objection regarding this change?

+1, for the reasons stated here.  SHOULD is a dangerous word, especially around APIs. :-)

Regarding the ContainerExceptionInterface:

I'd like to make the opposite move and switch from MUST to SHOULD, as in: "Exceptions directly thrown by the container SHOULD implement the ContainerExceptionInterface".
Furthermore, I'd like to emphasize the "directly" part. For instance, I understand perfectly that a cyclic dependency or a config file typo should throw an exception implementing ContainerExceptionInterface. However, if the user is using a container based on factories (like Disco or PimpleInterop), and if the factory written by the user is throwing an exception, I find it useless to wrap that exception in another exception implementing the ContainerExceptionInterface. To me, it adds noise and nothing more.

This is an interesting point.  In a sense it's the inverse of what we had in PSR-6, where we explicitly wanted to catch and swallow inner-system exceptions because a cache miss should not bring a system down hard. In this case, if some underlying implementation breaks (eg, file system not writable when writing out a compiled container) the system is going to be b0rked anyway so "fail fast" suggests the actual error should propagate up directly.

So I'd be +1 to the "directly" language, and flag this as something that in the future we should be mindful of to evaluate case-by-case for each PSR.

--Larry Garfield