Skip to content

[HttpKernel] Enhance exception message for services not found in dedicated service locators #25367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Dec 6, 2017

Q A
Branch? 3.4
Bug fix? yes (DX)
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #25342, #25196
License MIT
Doc PR -

Misses tests for now.
@weaverryan I may need your help to make the message better if you have any suggestions :)

image

@@ -150,7 +163,7 @@ private function handleRaw(Request $request, $type = self::MASTER_REQUEST)
$arguments = $event->getArguments();

// call controller
$response = call_user_func_array($controller, $arguments);
$response = \call_user_func_array($controller, $arguments);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improves perf, and DX, by removing one noisy frame in exception traces

@nicolas-grekas nicolas-grekas changed the title [KttpKernel] Enhance exception message for services not found in dedicated service locators [HttpKernel] Enhance exception message for services not found in dedicated service locators Dec 6, 2017
@curry684
Copy link
Contributor

curry684 commented Dec 6, 2017

I like the detection and how it works out, yet I'm not really sure it solves the DX issue we ran into at #25342.

I mean - the guy just followed the tutorial, derived from AbstractController as explicitly advised, and got an inexplicable error. From that background, the new error message, although 10 times more usable to experienced Symfony developers, isn't a single bit clearer to the less seasoned Symfony user, to them it boils down to:

The service you asked for exists, but not in [something you never asked for] of your [something you never expected or knew to exist]. Did your tutorial really not mention that underneath this base class we use an interface you'll likely never care or learn about with a static function you cannot easily override or extend to not fix this behavior? Because actually you should use DI but we don't have the context at where we throw this exception to be able to tell you that.

And I have no clue how to do this one better either, so I'm also hoping @weaverryan has a brilliant idea on this 😉

@nicolas-grekas
Copy link
Member Author

Don't forget that the goal of the message is not to provide full documentation, but mostly to accurately hint into the right direction.
Leaving the details to official doc + Google/SO is already the real solution to solve any errors.

@nicolas-grekas
Copy link
Member Author

Oh, and the second part of your comment, about the context, will be fixed on 4.1, once we will hide by default stack frames in vendor (and keep the one from src/ visible.)

@nicolas-grekas
Copy link
Member Author

Closing as I'm working on a more local approach.

@nicolas-grekas nicolas-grekas deleted the di-dx-locator branch December 7, 2017 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants