Page MenuHomePhabricator

find a better fix on LoadBalancer connection error
Closed, DeclinedPublic

Description

Bug T31233 has been fixed by Tim with r90266 in REL1_17 and r96517 in trunk.

The commit message ask for a cleaner way to handle a database connection error:

Temporary fix which roughly restores the 1.16 behaviour of openConnection(), returning a Database object with mOpened = false if the connection fails. However, the idea of throwing an exception from a constructor and then holding on to a reference to the constructed object by saving it in the exception object seems kind of icky. Needs a better fix in trunk.

Code snippet:

try {
    $db = DatabaseBase::newFromType( $server['type'], $server );
} catch ( DBConnectionError $e ) {
    // FIXME: This is probably the ugliest thing I have ever done to 
    // PHP. I'm half-expecting it to segfault, just out of disgust. -- TS    $db = $e->db;
}

Details

Reference
bz33036

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:06 AM
bzimport set Reference to bz33036.
bzimport added a subscriber: Unknown Object (MLST).

Antoine: Is this still valid, and would you work on this at some point?
If not, any idea who could tackle this?

This bug is merely a copy paste of what Tim said on review of r90266 ( https://www.mediawiki.org/wiki/Special:Code/MediaWiki/90266#c19929 ) so we do not forget about it. I have no plan to work on it nor I think it is important, maybe we will get it done one day.

Feel free to lower the priority.

Krinkle set Security to None.

There is already a FIXME in the code, there is not much point in keeping this task around.