[DX][HttpKernel] Throw a sensible exception when controller has been removed #25339
Conversation
01cbffb to
df6b238
Compare
| @@ -75,6 +75,8 @@ protected function createController($controller) | |||
| return $service; | |||
| } | |||
|
|
|||
| $this->throwExceptionIfControllerWasRemoved($controller); | |||
There was a problem hiding this comment.
L70 can also fail for the same reason
There was a problem hiding this comment.
Yes, I think it's a good idea to add a similar check there too. Currently you would get this error:
The "app.my_controller" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.
There was a problem hiding this comment.
But that's fine, it fails with the following exception:
The "app.my_controller" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.
There was a problem hiding this comment.
It feels inconsistent to me to get different errors depending on how you configured the route.
There was a problem hiding this comment.
Refactored it and added another DX improvement for missing __invoke methods at the same time 😉
73136df to
524e148
Compare
| if (!$this->container->has($controller)) { | ||
| $this->throwExceptionIfControllerWasRemoved($controller); | ||
|
|
||
| throw new \LogicException(sprintf('The controller "%s" do not exists.', $controller)); |
There was a problem hiding this comment.
Controller not found: service "%s" does not exist.
| // invokable controller in the "service" notation | ||
| return $service; | ||
| if (!method_exists($service, '__invoke')) { | ||
| throw new \LogicException(sprintf('The controller "%s" cannot be called without a method name. Did you forget an "__invoke" method?', $controller)); |
There was a problem hiding this comment.
Controller "%s" cannot [...]
524e148 to
458d63f
Compare
|
Yep, I like your wording suggestions @nicolas-grekas. Changed 👍 |
|
I'm a bit confused here, are you referring to missing controllers or missing actions? |
|
AFAIK "actions" are not an "official" Symfony thing. It's just a controller. |
|
Thank you @sroze. |
…ler has been removed (sroze) This PR was merged into the 3.4 branch. Discussion ---------- [DX][HttpKernel] Throw a sensible exception when controller has been removed | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25335 | License | MIT | Doc PR | ø Following on #25201, we need to throw the same kind of sensible exception when the controller service is not found. Commits ------- 458d63f Throw a sensible exception when controller has been removed
Following on #25201, we need to throw the same kind of sensible exception when the controller service is not found.