This error occurs when there are no environments to load. The code should more gracefully handle SQL errors as well as common conditions that cause the errors.

Steps to replicate the issue:

  • Connect to Acquia Search, set it to the default environment
  • Delete the other environment connected to the "localhost" environment
  • Set the "acquia_network_address" variable to something bogus, i.e. "http://example.com"
  • Run cron

Now your site is broken in many places, most commonly /admin/config pages.

CommentFileSizeAuthor
#12 1778150-12-D6.patch3.99 KBpwolanin
#11 1778150-11-D6.patch4.43 KBpwolanin
#5 1778150-5.patch2.74 KBpwolanin
#2 error-handling-1778150-2.patch6.4 KBcpliakas
#2 error-handling-whitespace-1778150-2.txt810 bytescpliakas
#1 apachesolr-fatal-error-1778150-1.jpg65.32 KBcpliakas

Comments

cpliakas’s picture

cpliakas commented
StatusFileSize
new apachesolr-fatal-error-1778150-1.jpg65.32 KB

Screenshot of the error message.

cpliakas’s picture

cpliakas commented
Status: Active » Needs review
StatusFileSize
new error-handling-whitespace-1778150-2.txt810 bytes
new error-handling-1778150-2.patch6.4 KB

The attached patch handles the error condition and wraps the SQL in a try / catch block to handle errors not yet discovered. There is also a whitespace patch to more easily show what the proposed changes are.

dankobiaka’s picture

dankobiaka commented

This error brought down my entire site this morning. I can confirm it doesn't just effect admin/config/* pages.

Can the patch be added ASAP?

pwolanin’s picture

pwolanin commented

I'm a bit confused by this - what's the underlying SQL that's failing? Catching the exception seems a bit bogus - we should make sure the SQL is valid - it's probably just the IN condition with array_keys($environments)

We should also get rid of the db_table_exists() call in that function and move the ones in apachesolr_load_all_environments() - that's a potentially large performance hit for no good reason.

pwolanin’s picture

pwolanin commented
StatusFileSize
new 1778150-5.patch2.74 KB
cpliakas’s picture

cpliakas commented

Catching the exception seems a bit bogus - we should make sure the SQL is valid

Yes, that is exactly what the patch in #1778150-2: Fatal error on all admin/config/* pages caused by SQL error in apachesolr_environment_load_subrecords() does. It handles the case that causes the invalid SQL. Then it adds a try / catch block just to make 100% sure a bad query doesn't happen so we don't find out about it by whitescreening.

anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
commented

Just want to confirm I'm having this issue too. For me it produced a non-recoverable WSOD (had to restore the DB) after disabling acquia_search.module because the "localhost" env was missing.

All details in dup: http://drupal.org/node/1782930

pwolanin’s picture

pwolanin commented

@Chris - we shouldn't need to try/catch every SQL query. Is there something in this one particularly that's causing issues? Is this called from hook_init() and that's causing the issue?

If so, we should try/catch there, and not in this API function.

cpliakas’s picture

cpliakas commented
Status: Needs review » Reviewed & tested by the community

@pwolanin, yeah, that's fine. I am honestly not sure where else it is called, but I just want to make sure we are handling any mistakes that we might not see right now gracefully. Clearly this issue is bigger than just your search pages, so I just want to be extra careful. The $environments variable is the one causing this bug, so if that is addressed then I am more than happy to have this committed and refine later.

Thanks for the patch,
Chris

pwolanin’s picture

pwolanin commented
Version: 7.x-1.x-dev » 6.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

committed. need to see if backport is required for 6.x-3.x

pwolanin’s picture

pwolanin commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 1778150-11-D6.patch4.43 KB

Here's a quick port to D6

pwolanin’s picture

pwolanin commented
StatusFileSize
new 1778150-12-D6.patch3.99 KB

Somehow an extra function was patched.

pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed

committed to 6.x, though I'm seeing other unrelated issues.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.