drupal_goto breaks cron.php [#453286]

After installing this module I realized that my cron was broken and after searching other posts I found the culprit was drupal_goto.

Comment	File	Size	Author
#3	uc_webform_productize-453286.patch	1012 bytes	jdwfly

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jdwfly CreditAttribution: jdwfly commented 4 May 2009 at 16:13

Status:

Active

» Needs review

Here is the patch

Comment #2

raintonr CreditAttribution: raintonr commented 17 May 2009 at 10:27

Project:	Ubercart Webform Productize	» Drupal core
Version:	6.x-1.x-dev	» 6.12
Component:	Code	» php.module
Status:	Needs review	» Active

Re: #1 - sorry, but I don't see any patch here.

I'm posting this as I spent a few frustrating hours this morning due to the very same problem. IMHO this is a core issue (not really limited to Ubercart) so also changed the project to 'core' as drupal_goto should not 'fire' on a node_load due to cron or any other similar situation.

The site I am working on needs a 'members' page that shows the login screen if the user isn't logged in and re-directs to the forum if they are. Easy (or so I thought), using php_filter placed this in a node on that path:

<?php
  global $user;

  if ($user->uid) {
    drupal_goto("forum");
  } else {
    drupal_goto("user");
  }
?>

Sadly, this caused cron to keep repeating this error in the logs, "Cron run exceeded the time limit and was aborted". Visiting cron.php would end up at the forum page rather than coming up blank. This was happening instantly, certainly not after the time limit had passed as the log file states. Note: at this time I hadn't made the connection between the page being displayed (the forum) and the error.

After lots and lots of debugging traced this to the node with the goto in and saw the issue. node_load even fails for this type use so perhaps this is not limited to cron.

Anyhow - here's hoping someone else will find this useful and perhaps this could be resolved in core.

P.S. In the end replaced the node in question with a custom module on the same path as I didn't know how to fix this without such a move.

Comment #3

jdwfly CreditAttribution: jdwfly commented 18 May 2009 at 15:52

Project:	Drupal core	» Ubercart Webform Productize
Version:	6.12	» 6.x-1.x-dev
Component:	php.module	» Code

File	Size
uc_webform_productize-453286.patch	1012 bytes

My patch was there...
Here it is again.

This is not an issue with Drupal core. Please search this out before you change the project information. When you run a drupal goto you must make sure that you exclude cron from doing it. When cron is run it also indexes the pages for the search engine. This means hook_nodeapi is loaded and at the very beginning of that is a drupal goto. You should have also noticed that this module adds a product node but if you try to actually go to that node you get redirected to it's associated content type. This is where the drupal goto is working. When running cron manually you will notice that you get redirected to the first Ubercart Webform Product node you created. This is because of the drupal goto is sending cron to that page.

Comment #4

jdwfly CreditAttribution: jdwfly commented 18 May 2009 at 15:53

Status:

Active

» Needs review

Comment #5

raintonr CreditAttribution: raintonr commented 19 May 2009 at 11:34

No offence, but wouldn't you have thought it would be better to change drupal_goto itself to perform your check?

That way all code and modules that may be affected in future will be saved the trouble.

I would have also thought that if done in core drupal_cron_run could set a global variable that could be more reliably checked than $_SERVER['SCRIPT_NAME'].

Comment #6

jdwfly CreditAttribution: jdwfly commented 20 May 2009 at 19:17

You may be right about needing to change this in core, but Drupal 6 core is not going to change this. If you want it changed in Drupal 7 core then I suggest you start an issue on it and not hijack this issue with this module. This issue isn't here to argue whether we should change core, but rather how this module should be changed so that it doesn't break the core.

You could use this as proof that it needs to be changed in core though. There are also a few other modules where I have seen this problem and it was solved by checking the $_SERVER['SCRIPT_NAME']. Personally, I don't know whether or not core needs to be changed, but it does seem to be a problem with some modules using drupal_goto.