Laravel – variadic parameter trap
What do you expect from the framework if the dependency cannot be created? Sure, it should be an exception, but I had an unpleasant surprise.
Problem
I had code similar to the following:
final class Service { /** * @var HandlerInterface[] */ private readonly array $handlers; public function __construct( HandlerInterface ...$handlers, ) { $this->handlers = $handlers; } public function doSomething(string $type): void { foreach ($this->handlers as $handler) { if ($handler->supports($type)) { $handler->handle(); } } } }
And the proper configuration in the ServiceProvider:
$this ->app ->when(Service::class) ->needs(HandlerInterface::class) ->giveTagged(HandlerInterface::class) ;
Then I decided to refactor ServiceProvider, where the handler was defined, to DeferrableProvider and unfortunately made a mistake in the provides method. It caused Laravel couldn’t find the definition of HandlerInterface. In that case, I would expect an exception, and there was the exception under the hood – BindingResolutionException, but the container of Laravel has a catch as follows:
\Illuminate\Container\Container::resolveClass
catch (BindingResolutionException $e) { if ($parameter->isDefaultValueAvailable()) { array_pop($this->with); return $parameter->getDefaultValue(); } if ($parameter->isVariadic()) { array_pop($this->with); return []; } throw $e; }
This means that even if the dependency cannot be created and a variadic parameter is used it will bind an empty array by default. This is quite odd and rather doesn’t seem to be in line with the Fail Fast principle. Also, my integration tests didn’t catch this bug, because it was related to communication between two modules, and according to the Modular Monolith architecture, I just bind a fake object to my tests.
Improvement
use Webmozart\Assert\Assert; final class Service { /** * @var HandlerInterface[] */ private readonly array $handlers; /** * @param HandlerInterface[] $handlers */ public function __construct( array $handlers, ) { Assert::allIsInstanceOf($handlers, HandlerInterface::class); Assert::notEmpty($handlers); $this->handlers = $handlers; } public function doSomething(string $type): void { foreach ($this->handlers as $handler) { if ($handler->supports($type)) { $handler->handle(); } } } }
$this ->app ->when(Service::class) ->needs('$handlers') ->giveTagged(HandlerInterface::class) ;
The improvement is quite simple, we can just throw an exception as fast as possible when something unexpected happens e.g. $handlers array is empty.
Subscribe and master unit testing with my FREE eBook (+60 pages)! 🚀
In these times, the benefits of writing unit tests are huge. I think that most of the recently started projects contain unit tests. In enterprise applications with a lot of business logic, unit tests are the most important tests, because they are fast and can us instantly assure that our implementation is correct. However, I often see a problem with good tests in projects, though these tests’ benefits are only huge when you have good unit tests. So in this ebook, I share many tips on what to do to write good unit tests.
Leave a Reply