Skip to content
This repository has been archived by the owner on Feb 6, 2020. It is now read-only.

Catch statement in service manager should handle any exception code #41

Open
nuxwin opened this issue Oct 13, 2015 · 11 comments
Open

Catch statement in service manager should handle any exception code #41

nuxwin opened this issue Oct 13, 2015 · 11 comments

Comments

@nuxwin
Copy link

nuxwin commented Oct 13, 2015

Hello ;

I've a service factory like this:

/**
 * Class DatabaseServiceFactory
 * @package iMSCP\Service
 */
class DatabaseServiceFactory implements FactoryInterface
{
    /**
     * Create database service
     *
     * @param ServiceLocatorInterface $serviceLocator
     * @return Database
     */
    public function createService(ServiceLocatorInterface $serviceLocator)
    {
        $config = Registry::get('config');
        $imscpDbKeys = new ConfigFileHandler($config['CONF_DIR'] . '/imscp-db-keys');

        if (!isset($imscpDbKeys['KEY']) || !isset($imscpDbKeys['IV'])) {
            throw new \RuntimeException('imscp-db-keys file is corrupted.');
        }

        $db = Database::connect(
            $config['DATABASE_USER'],
            Crypt::decryptRijndaelCBC($imscpDbKeys['KEY'], $imscpDbKeys['IV'], $config['DATABASE_PASSWORD']),
            $config['DATABASE_TYPE'],
            $config['DATABASE_HOST'],
            $config['DATABASE_NAME']
        );

        $db->execute('SET NAMESS `utf8`');

        return $db;
    }
}

Here, if an error occurs, a PDOException is throw. The problem is that the underlying catch statement from the service manager don't handle PDOException code and thus, the following fatal error is raised:

Fatal error: Wrong parameters for Exception([string $exception [, long $code [, Exception $previous = NULL]]]) in /var/cache/imscp/packages/vendor/zendframework/zend-servicemanager/src/ServiceManager.php on line 950

Note: Here the wrong query SET NAMESS is intentional.

@manuakasam
Copy link

Whole story to this point is writte in the first comment here: http://php.net/manual/en/class.pdoexception.php

Question to be asked is:
Should ZF ensure a type-casting to int on all exceptions? (who knows where people might end up using PDO)? I don't think that an cast to int hurts performance much and everyplace we do that'd be an error case anyways so performance doesnt matter.

@nuxwin
Copy link
Author

nuxwin commented Oct 13, 2015

@manuakasam

As I've said on IRC, I'll not loose my time with PHP dev team by reporting bugs on their bug tracker due to their EGO (and so on...).

For the record

In short: The PHP dev team does not hesitate to violate its own interfaces (by laziness?)

@Ocramius
Copy link
Member

(by laziness?)

Not really, it's mostly because many of the core members of PHP-DEV are still attached to very old practices that aren't really considered "good" anymore.

@spengilley
Copy link

This looks like it has been forgotten. Is this still an issue to resolve?

@nuxwin
Copy link
Author

nuxwin commented Apr 3, 2016

@spengilley

You mean in ZF3? I don't know. Even through, we can handle this problem locally by catching the exception.

@spengilley
Copy link

@nuxwin Thanks for responding.

I was talking specifically in ZF2.

I am looking for some easy issues to fix. I am new to contributing to open source :)

@nuxwin
Copy link
Author

nuxwin commented Apr 3, 2016

@spengilley

Not the best issue to start ;) From my point of view, this issue can be closed.

@Ocramius ?

@lucian303
Copy link

Why would you catch any exceptions in the service manager and re-throw generic, useless exceptions instead of leaving the proper exceptions to bubble up and be caught as needed by the user? If I had a penny for everytime the useless exceptions from the service manager were thrown and I had no clue what went wrong, I'd be rich. Maybe you should reconsider this in the future.

@Ocramius
Copy link
Member

Ocramius commented Aug 8, 2018

You can rely on this type from a caller in order to wrap the DIC and write fallback logic.

If everything was a \Exception we may as well disable the exception system overall 😛

If the problem is dealing with "previous exceptions", then my suggestions is to get up to speed with that concept, and quickly, since most good OO libraries out there do wrap exceptions in known failure scenarios.

@Ocramius
Copy link
Member

Ocramius commented Aug 8, 2018

Anyway, if anybody wants to pick this up, write a test with a factory that does something like this:

(function (Potato $foo) {})();
```

Then verify that the type of the error is indeed a `ServiceNotCreated`.

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-servicemanager; a new issue has been opened at laminas/laminas-servicemanager#26.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants