backdrop-contrib / backdrop_upgrade_status

Checks to see if the installed modules on a Drupal 7 site are ready for upgrade to Backdrop CMS.
1 stars 5 forks source link

Cron broken: Warning: Cannot modify header information - headers already sent #16

Open philsward opened 3 years ago

philsward commented 3 years ago

Getting an error: Warning: Cannot modify header information - headers already sent when trying to manually run BUS on one of my sites.

I think it's generating an error per module installed.

It was working at one point, but now it is not which I know isn't helpful...

image

I will try to track down any offenders, however it may be related to issue: https://github.com/backdrop-contrib/backdrop_upgrade_status/issues/12

Update: Later discussions with myself will reveal this isn't a header problem as much as it is a problem with how BUS is fetching info after the first time.

Reproduce: 1) Install BUS 2) Manually or through cron, check for the status within BUS (admin/reports/updates/backdrop-upgrade) 3) Run cron and witness errors 4) Run manual check and witness errors 5) Disable BUS 6) Run cron (should run successfully this time) 7) Re-Enable BUS 8) Run Cron or check manually

Might have to completely uninstall BUS if disabling and running cron doesn't re-condition things.

philsward commented 3 years ago

This apparently also broke cron.

Uninstalling BUS and re-installing it has fixed the cron issue and now BUS is also working correctly. Will monitor site for the problem described in issue #12

philsward commented 3 years ago

Turns out I'm still having the same problem.

It appears that you can only check the first time only. After that, it corrupts "something" and won't re-check and causes cron to not run properly.

philsward commented 3 years ago

I've noticed that part of the reason cron hangs up is because BUS is apparently trying to check for status on every cron run. Since BUS is hanging up, cron can't complete.

The only way to fix this is to uninstall BUS and re-install it OR disable BUS, run cron then re-enable BUS. The very first check will be successful but every check afterwards will error out and prevent cron from running.

It acts like BUS is first-in-line when cron runs and I am not seeing any watchdog messages that cron is successful.

This should be changed to either: 1) Only run weekly 2) Be configurable by the end-user with the default set to weekly.

Example after a cron run or manual BUS check image

laryn commented 3 years ago

I'm seeing this too.

herbdool commented 3 years ago

When you use hook_cron it will run every cron run. But I suggest using elysia_cron if you don't want it running each time.

By the way, it seems odd to me to have this module installed for a long time. Isn't just a temporary thing?

herbdool commented 3 years ago

It would be better to figure out why it's failing on cron and debug that instead.

laryn commented 3 years ago

By the way, it seems odd to me to have this module installed for a long time. Isn't just a temporary thing?

I'd guess that many site builders who install this won't think to uninstall it after checking, or will leave it there to check back on modules periodically. I agree, figuring why it's failing on cron and fixing that seems like the right solution.

herbdool commented 3 years ago

Looks like hook_cron was added after it was forked from update_status. I'm not sure if it adds much value. Why would someone want this to run on cron at all? I propose we remove this functionality altogether.

herbdool commented 3 years ago

The other part is in the install file. In upgrade_status.install it looks like:

/**
 * Implements hook_install().
 */
function upgrade_status_install() {
  $queue = DrupalQueue::get('upgrade_status_fetch_tasks', TRUE);
  $queue->createQueue();
}

/**
 * @addtogroup updates-6.x-to-7.x
 * @{
 */

/**
 * Create a queue to store tasks for requests to fetch available update data.
 */
function upgrade_status_update_7000() {
  module_load_include('inc', 'system', 'system.queue');
  $queue = DrupalQueue::get('upgrade_status_fetch_tasks');
  $queue->createQueue();
}

This module doesn't have the update hook - which isn't necessary but wondering why the install hook isn't also loading module_load_include('inc', 'system', 'system.queue'); Can't hurt to add the update hook here and see if that fixes anything.

herbdool commented 3 years ago

I've made sure the queue is created here https://github.com/backdrop-contrib/backdrop_upgrade_status/commit/5747dba51aaff7e4ad787e1112c6494b2a1a00c1

As for removing cron I'll wait a day or two until I hear if anyone has objections.

laryn commented 3 years ago

No objections from me -- if it's not in update_status I don't see a need to add it.

philsward commented 3 years ago

Objection

Long term, I want to get telemetry into that module that can report back the list of installed modules. I have not found anyone who is willing to spend the time with me to hammer it out.

I've detailed the need in another issue on this queue (on phone so hard to grab #) but, it would ideally need to run cron at least once per week.

Next, this is a module that needs to remain installed until the day of migration. There are still a lot of modules that have not been ported or have not been added to core. D7 users need the ability to check back on a periodic basis to see if their site is any closer to a migration path.

My recommendation is to figure out why it's choking on cron.

jenlampton commented 3 years ago

It's also helpful to have this installed on a site, and check it every month or so to see how close the site is to being "ready" to port. We don't need to check for new updates every hour though, so maybe we should add our own timer for the last time we checked, and only check once a week?

@philsward and @laryn I'm unable to reproduce this cron issue. I've got the module running on 3 sites now without problems. Are you also seeing the failure it on a fresh install? If not, what other modules do you have on your sites?

jenlampton commented 3 years ago

I'm unable to reproduce this cron issue

I can reproduce it now. Working on a fix...

roxy-jaqkar commented 2 years ago

Please push the reports version.