suncat2000 / MobileDetectBundle

Symfony 2/3/4 bundle for detect mobile devices, manage mobile view and redirect to the mobile and tablet version.
397 stars 154 forks source link

InactiveScopeException: You cannot create a service ("request") of an inactive scope ("request"). #12

Closed MatTheCat closed 11 years ago

MatTheCat commented 11 years ago

When I try to load my website after installing this bundle I get the message InactiveScopeException: You cannot create a service ("request") of an inactive scope ("request").

Any idea?

suncat2000 commented 11 years ago

What version of symfony?

MatTheCat commented 11 years ago

I'm currently on Symfony 2.1.4 (sorry if my english is bad).

suncat2000 commented 11 years ago

My english is bad too :) I plan to working on MobileDetectBundle this weekend, make some changes, also check out your problem

MatTheCat commented 11 years ago

Yay thanks =)

suncat2000 commented 11 years ago

Hi, I could not check the version 2.1.4-DEV, since it does not install through the composer, and to organize vendors manualy do not have the time.

Checked with version 2.1.3 all ok! I did a little refactoring. Now uses the original class of Mobile_Detect from serbanghita. Check now. Perhaps the problem will magically disappear =)

iamdey commented 11 years ago

Actually there is always a problem with some of your services when someone tries to launch a command in CLI. For example, sometimes you will get this error when launching the simple app/console cache-clear.

This is due (obviously) to a request to an inactive scope ... I've experienced myself on other projects.

Look at that lines : https://github.com/suncat2000/MobileDetectBundle/blob/master/Helper/DeviceView.php#L50 and https://github.com/suncat2000/MobileDetectBundle/blob/master/EventListener/RequestListener.php#L56-66

With that line you tell to symfony to get the request that doesn't exist in CLI

public function __construct(Container $serviceContainer)
{
//...
$this->request = $serviceContainer->get('request');
}

Well you there's two solutions (I'm not sure of that) :

mobile_detect.request_listener:
        class: %mobile_detect.request_listener.class%
        arguments: [ @request ]
        scope: request
protected function getRequest()
{
    if ($this->container->isScopeActive('request')) {
         return $this->container->get('request');
    }
}

I would make a PR unfortunately I have no time today

suncat2000 commented 11 years ago

Yes, the problem was related to the Request. In this bundle, mobile_detect.twig.extension use mobile_detect.device_view service which uses Request. But in this refactor, I add check scope in DeviceView class. It solved the problem for me

php app/console cache:clear - execute without errors (sf2.1.3)

Update this vendor Note: New version use original Mobile_Detect class from @serbanghita repository

iamdey commented 11 years ago

woops, shame on me I failed on reading the constructor of device view.

But because of $this->container->get('mobile_detect.request_listener'); is allowed in any part of code, it might be more elegant to check the scope is active, isn't it?

suncat2000 commented 11 years ago

Yes, but why do you need in CLI mode a RequestListener? If a Request in the CLI mode is missing.

Here, the main problem with the Twig, because it is used in CLI mode (to work with templates).

php app/console cache:clear too uses Twig.

Because mobile_detect.twig.extension uses mobile_detect.device_view, and it in turn uses the Request and we get this exception. If check scope is active in mobile_detect.device_view, Request can be excluded from mobile_detect.twig.extension and all OK.

If we follow your logic, then you have to check the scope of Request in all services where uses Request? I think that if you get mobile_detect.request_listener in the CLI mode, then you should get an exception:

InactiveScopeException: You cannot create a service ("request") of an 
inactive scope ("request").

or another...

iamdey commented 11 years ago

Well it is just a purpose to get your bundle implemented with the best practices and I asking myself too because I don't mastering yet Dependency Injection and I'm not sure what generates and how generates the commands app/console cache:clear --env=prod or Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap.

For now I don't need to get RequestListener in a console command but someone may wants to reused some code or faking things and use it.

The only things I know is the doc advises to not store the request service in a service property except for certain cases.

suncat2000 commented 11 years ago

Ok You propose to replace in RequestListener $this->request for $this->container->get('request') or something similar? And add property container to RequestListener? I understand you correctly?

suncat2000 commented 11 years ago

@MatTheCat you can update MobileDetectBundle and check the problem?

MatTheCat commented 11 years ago

I just updated to the latest version and I get no error with the default config. I will test this later =)

MatTheCat commented 11 years ago

Now I get:

ErrorException: Notice: Trying to get property of non-object in /var/www/Plyce-Server/vendor/suncat/mobile-detect-bundle/SunCat/MobileDetectBundle/Helper/DeviceView.php line 117

It seems DeviceView $request property is null because $serviceContainer->isScopeActive('request') returns false. Any idea?

iamdey commented 11 years ago

I'm curious on how did you get this error. Can you give the context (cli or web page / implementation) and the stacktrace?