apachesolr_get_solr() throws exceptions when the default environment stored in the apachesolr_default_environment is not a valid environment; this can happen if the default environment is deleted programmatically or if the variable is unset and is allowed to default to 'solr' when the default 'solr' environment has been deleted.

A site is probably doing something wrong if this variable is not present and set to a valid environment, but it can potentially happen with Features and exportable environments. I get the impression that this function is supposed to throw an exception, and that callers are expected to wrap it in a try-catch, but there are several functions that do not do this. Should all uses of apachesolr_get_solr() be wrapped in try-catch blocks? If so, I'd be happy to write a patch...

Comments

becw’s picture

Title: Uncaught exception when the default environment does not exist » Uncaught exception when the default environment does not exist (patch)
Category: support » bug
Status: Active » Needs review
StatusFileSize
new6.11 KB

Here's a patch that wraps the remaining calls to apachesolr_get_solr() in try-catch blocks, and throws an exception when there is no default environment configured.

nick_vh’s picture

I can see the use of this. Are you sure these are all the apachesolr_get_solr calls?
Would it be easier if we make this try catch in apachesolr_get_solr itself?

becw’s picture

Doing the try-catch in apachesolr_get_solr() would mean that it only needs to happen in one place, but all of the calls to that function rely on getting a DrupalApacheSolrService object back. If we changed apachesolr_get_solr(), everything that currently uses it would have to be updated to handle getting FALSE instead of an object... so I don't think it would necessarily be easier.

pwolanin’s picture

looks reasonable, espeically if the function is already documented to throw an exception

nick_vh’s picture

Status: Needs review » Fixed

Committed! Gave you credit for the patch ;-)

Thanks

nick_vh’s picture

Status: Fixed » Patch (to be ported)
becw’s picture

Awesome, thanks!

pwolanin’s picture

Version: 7.x-1.x-dev » 6.x-3.x-dev
nick_vh’s picture

StatusFileSize
new7.44 KB

Commited to 6.x-3.x

nick_vh’s picture

Status: Patch (to be ported) » Fixed
nick_vh’s picture

Status: Fixed » Closed (fixed)