Skip to content
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

command scheduler not working in production environment #955

Closed
fritzmg opened this issue Jul 12, 2017 · 23 comments
Closed

command scheduler not working in production environment #955

fritzmg opened this issue Jul 12, 2017 · 23 comments
Assignees
Labels
Milestone

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Jul 12, 2017

It appears in Contao 4.4 the command scheduler is not working. CommandSchedulerListener::canRunController always returns false, because of

in_array($request->attributes->get('_route'), ['contao_backend', 'contao_frontend'], true)

The ParameterBag from $request->attributes does not contain the _route (anymore). Try with

var_dump(in_array($request->attributes->get('_route'), ['contao_backend', 'contao_frontend'], true));
var_dump($request->attributes->get('_route'));
var_dump($request->attributes);

within CommandSchedulerListener::canRunController:

bool(false)
NULL
object(Symfony\Component\HttpFoundation\ParameterBag)#12 (1) {
  ["parameters":protected]=>
  array(0) {
  }
}
@leofeyer
Copy link
Member

The ParameterBag from $request->attributes does not contain the _route (anymore).

Yes, it does:

@leofeyer
Copy link
Member

I just tested it and it works like a charm on my system.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

Hm, I made another clean installation with Contao 4.4.1. Everything in my initial post still holds true. Not sure why it would be different in my installation.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

The full dump of the Request object within onKernelTerminate also does not contain any attributes:

object(Symfony\Component\HttpFoundation\Request)#9 (24) {
  ["attributes"]=>
  object(Symfony\Component\HttpFoundation\ParameterBag)#12 (1) {
    ["parameters":protected]=>
    array(0) {
    }
  }
  …
}

@leofeyer
Copy link
Member

Why don't you add dump() to the FrontendCron::run() method? Then you will see that it works.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

Why don't you add dump() to the FrontendCron::run() method? Then you will see that it works.

I tried that, but as expected there is no output, since the FrontendCron is never run. Also the tl_cron table stays empty/is not updated.

@leofeyer
Copy link
Member

It works like a charm on my system. The cause of the problem must be somewhere else.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

The results are different in the prod and dev environment. While

var_dump($request->attributes->get('_route'));

in prod results in NULL,

dump($request->attributes->get('_route'));

results in "contao_index".

Try the following:

  1. TRUNCATE tl_cron.
  2. Open front end via app.php.
  3. Check if cron was run.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 12, 2017

Ah yes, I see the problem now. This only happens, if the contao_index route is used.

Reproduction

  1. Set prepend_locale to false.
  2. Clear the cache.
  3. TRUNCATE tl_cron.
  4. Set the alias of your start page to index.
  5. Open the start page (i.e. http://example.org/) in the front end via app.php (prod).

The FrontendCron will not be run (verify that tl_cron stays empty), since the ParameterBag of the Request for the contao_index route will be empty.

Repeat 5. again with app_dev.php (dev). The FrontendCron will still not be run, since the name of the route is contao_index.

@fritzmg fritzmg changed the title command scheduler not working command scheduler not working on the index page Jul 12, 2017
@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 13, 2017

@leofeyer I have analyzed it a bit more. Replace

// Handle the request
$request = Request::createFromGlobals();
$response = $kernel->handle($request);
$response->send();
$kernel->terminate($request, $response);

with

// Handle the request
$request = Request::createFromGlobals();
var_dump($request->attributes);
$response = $kernel->handle($request);
$response->send();
var_dump($request->attributes);
$kernel->terminate($request, $response);

for example and the look at the output from both the prod and dev environment on the index page.

Within the prod environment it will be:

object(Symfony\Component\HttpFoundation\ParameterBag)#12 (1) {
  ["parameters":protected]=>
  array(0) {
  }
}
…
object(Symfony\Component\HttpFoundation\ParameterBag)#12 (1) {
  ["parameters":protected]=>
  array(0) {
  }
}

Within the dev environment it will be:

object(Symfony\Component\HttpFoundation\ParameterBag)#70 (1) {
  ["parameters":protected]=>
  array(0) {
  }
}
…
object(Symfony\Component\HttpFoundation\ParameterBag)#70 (1) {
  ["parameters":protected]=>
  array(7) {
    ["_token_check"]=>
    bool(true)
    ["_controller"]=>
    string(60) "Contao\CoreBundle\Controller\FrontendController::indexAction"
    ["_scope"]=>
    string(8) "frontend"
    ["_locale"]=>
    string(2) "en"
    ["_route"]=>
    string(12) "contao_index"
    ["_route_params"]=>
    array(3) {
      ["_token_check"]=>
      bool(true)
      ["_scope"]=>
      string(8) "frontend"
      ["_locale"]=>
      NULL
    }
    ["_firewall_context"]=>
    string(38) "security.firewall.map.context.frontend"
  }
}

So for some reason, the attributes within the $request object that is passed to $kernel->terminate (which will trigger the onKernelTerminate function of the CommandScheduler down the line) will stay empty when the HttpCache kernel is in use.

@Toflar
Copy link
Member

Toflar commented Jul 14, 2017

@fritzmg can you try to require "terminal42/header-replay-bundle": "dev-hotfix/1.0.2 as 1.0.0" to see if that fixes the issue?

@leofeyer leofeyer added bug and removed invalid labels Jul 14, 2017
@leofeyer leofeyer added this to the 4.4.2 milestone Jul 14, 2017
@leofeyer leofeyer reopened this Jul 14, 2017
@leofeyer
Copy link
Member

I can confirm that it does not fix the issue. 😢

Also, I don't think it is the preflight request that is causing the issue. It neither works with the regular AppCache class. (But it sure does work with no cache at all.)

@leofeyer
Copy link
Member

leofeyer commented Jul 14, 2017

This is the request object without cache:

And this is the request object with cache:

Both screenshots show the CommandSchedulerListener::canRunController() method.

@aschempp
Copy link
Member

Looks like someone is deleting the attributes? What does it look like in 4.3?

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 14, 2017

May be it's a general problem and not related to Contao at all. I only found this obscure related issue though: https://stackoverflow.com/questions/15955373/symfony2-why-is-the-request-passed-to-a-kernel-terminate-eventlistener-affected

@leofeyer
Copy link
Member

At least I found out where the problem occurs.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L421

In the fetch() method, Symfony does a $subRequest = clone $request; and passes the $subRequest variable to the forward() method, so all the request attributes end up on the $subRequest object instead of the $request object.

    protected function fetch(Request $request, $catch = false)
    {
        $subRequest = clone $request;

        // send no head requests because we want content
        if ('HEAD' === $request->getMethod()) {
            $subRequest->setMethod('GET');
        }

        // avoid that the backend sends no content
        $subRequest->headers->remove('if_modified_since');
        $subRequest->headers->remove('if_none_match');

        $response = $this->forward($subRequest, $catch);

        if ($response->isCacheable()) {
            $this->store($request, $response);
        }

        return $response;
    }

I don't know why it is necessary to work on a cloned object here? @fabpot or @avalanche123 can you shed some light on this please?

@aschempp
Copy link
Member

@leofeyer you mean the request stored in the cache does not have the attributes? Maybe it does not really make sense to run the kernel.terminate event on a cache hit anyway?

@leofeyer
Copy link
Member

you mean the request stored in the cache does not have the attributes?

Exactly.

Maybe it does not really make sense to run the kernel.terminate event on a cache hit anyway?

Probably. However, if all FE pages are cached, the command scheduler would never be triggered.

@fritzmg fritzmg changed the title command scheduler not working on the index page command scheduler not working in production environment Jul 17, 2017
@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 17, 2017

This happens in the production environment in any case, regardless of whether you are using the page cache or not.

@leofeyer
Copy link
Member

Also see: symfony/symfony#23546

@leofeyer
Copy link
Member

A simple fix as suggested by @aschempp would be to revert the changes from fd36131 (see #736). @fritzmg Any objections?

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 24, 2017

Well my objection is only that it's not pretty when that error can be visible again at the bottom of the page. Though I think this does not happen anymore anyway due to the changes from 9fe4382, right?

@leofeyer leofeyer self-assigned this Jul 24, 2017
@leofeyer
Copy link
Member

Fixed in b5d2178.

agoat pushed a commit to agoat/contao-core-bundle that referenced this issue Aug 1, 2017
@leofeyer leofeyer modified the milestones: 4.4.2, 4.4 May 14, 2019
leofeyer added a commit that referenced this issue Nov 12, 2019
Description
-----------

As suggested in #722

Commits
-------

95f0c022 Ignore bugfix/* and feature/* branches on push (see #960)

Description
-----------

This will prevent the GitHub action to be run twice for pull requests (one time for the push and one time for the pull request).

Commits
-------

7dbdb6c0 Ignore bugfix/* and feature/* branches on push
e47dea80 Do not require league/uri anymore (see #722)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants