theorchard / monolog-cascade

Configure multiple loggers and handlers in the blink of an eye
MIT License
145 stars 62 forks source link

PHP error: "Cannot determine default value for internal functions ..." #37

Closed kafene closed 8 years ago

kafene commented 8 years ago

Example config:

handlers:
    mongodb:
        class: Monolog\Handler\MongoDBHandler
        mongo:
            class: MongoClient
            server: mongodb://example.com:27017
        database: doctrine
        collection: logs
loggers:
    doctrine:
        handlers: [mongodb]

Which results in the following error:

ReflectionException: Cannot determine default value for internal functions in /path/to/vendor/theorchard/monolog-cascade/src/Config/Loader/ClassLoader/Resolver/ConstructorResolver.php on line 105

I managed to get it working alrght:

https://github.com/kafene/monolog-cascade/commit/429ebaa3b8c2cfd25e985a5d4e84c886ddb66b7e

I'm not sure if this is the most appropriate fix, let me know if you'd like a PR.

rantonmattei commented 8 years ago

Hi, thanks for bringing this up. That's a good catch!

This is a Reflection bug. That's unfortunate... I found those 2 threads that you might have come across as well: https://bugs.php.net/bug.php?id=28114 https://bugs.php.net/bug.php?id=50798 The second one has more explanations about why they actually can't implement it. It looks like it won't be fixed unfortunately. :-( Ah, just seeing that they also documented it in the doc. http://php.net/manual/en/reflectionparameter.getdefaultvalue.php

Sure, feel free to shoot a PR. Thanks!

kafene commented 8 years ago

I realized that what your code is trying to do is provide every value, because there's no way to do, for example:

$foo = function ($number, $array = [1,2,3], $boolean = true) {};
$args = [5, default, false];
call_user_func_array($foo, $args);

There was an RFC for this - https://wiki.php.net/rfc/skipparams - but it didn't pass. There is simply no way to tell PHP that you want to leave a default value for one positional argument, but provide a user-defined value for a later one, even for user defined functions, but at least the Reflection API makes it possible to get the default value in that case.

MongoClient is actually a good example of why the solution I initially gave won't work, because it has a middle argument which is an array, and the default value is ['connect' => true], not just an empty array, so if I want to provide that driver_options argument, the only way to "skip" the options argument is to provide that default value explicitly, for non-internal functions, this is the case.

After giving it much thought, I think the best fix at the moment is to check if the default value is available (ReflectionParameter::isDefaultAvailable() returns false for optional arguments which are internal) and if not, just set it to null and hope for the best. If null is a valid default value, then everything will work fine, if not, it crashes, and the user hopefully recognizes what's happened and goes to fill in the real default value.