-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Throw useful error msg when trying to access not-found but private service #24444
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
Comments
However, this would force us to keep a list of private ids in the dumped container (including removed/inlined services, that we don't even have today at all). What about the memory usage here (although it should be possible to keep this array in shared memory if being careful) |
Correct. I think that it wouldn't matter if we put that list in a separate file (at least when using the multi-files dumped container.) |
So is this the final decision of the core team? Either way, I also think it's a DX requirement. @nicolas-grekas What do you mean by a separate file? We already have the |
That's the current state yes, so it should stay: this is just the final logical step of the work put by the community in this area About the list of private services, yes we can put it in the class for sure. The discussion was about finding a way to not put it there, in order to not bloat the container. |
That's fair enough, let's go this way. We need to write some great documentation/articles about how this is important then I guess. Sure, I get that it would be nice to move it somewhere else for performance reasons but for now I guess we can just re-use these |
…rvices (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Throw accurate failures when accessing removed services | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24444 | License | MIT | Doc PR | - See linked issue. This will throw a useful message when accessing a removed service. When setting a removed service, a deprecation notice will be thrown also so that in master we can throw an exception then. Commits ------- fe7f26d [DI] Throw accurate failures when accessing removed services
Right now, when a service is not found, we throw a
ServiceNotFoundException
with some suggestions.But when the lookup is for a private service that ended up being removed or inlined, that message is not accurate: we should instead hint about the service being private + suggest to either turn it public or not use the container at all (similar to the deprecation in
Container::get()
).On 4.0, where services will be private by default, this looks like a DX requirement to me, so we should do it during the feat. freeze.
The text was updated successfully, but these errors were encountered: