Development

#3529 (sfPropelFinderPlugin crashes application when SF_DEBUG === FALSE)

You must first sign up to be able to contribute.

Ticket #3529 (closed defect: fixed)

Opened 3 months ago

Last modified 2 months ago

sfPropelFinderPlugin crashes application when SF_DEBUG === FALSE

Reported by: tonypiper Assigned to: francois
Priority: major Milestone: plugins
Component: sfPropelFinderPlugin Version:
Keywords: Cc:
Qualification: Unreviewed

Description

When sfPropelFinderPlugin is installed, any frontend controller script that sets SF_DEBUG = false fails, logging the following PHP error in the webserver logs.

PHP Fatal error: Call to undefined method MySQLConnection::getLastExecutedQuery() in /Users/webdev/application-nam/trunk/plugins/sfPropelFinderPlugin/lib/sfPropelFinder.php on line 78

Line 74 onwards: 

  public function updateLatestQuery()
  {
    $this->latestQuery = Propel::getConnection()->getLastExecutedQuery();
  }

commenting out line 76 allows the application to run.

Attachments

patch.diff (0.9 kB) - added by w.piekutowski on 06/03/08 12:39:01.
Improve checking if debug mode is enabled and exceptions usage

Change History

06/03/08 12:38:10 changed by w.piekutowski

  • summary changed from sfPropelFinderPlugin crashes application when SF_DEBUG = false to sfPropelFinderPlugin crashes application when SF_DEBUG === FALSE.
  • version deleted.
  • milestone set to plugins.

This was fixed in r8212.

I'd argue if this is a good fix because it doesn't really show the intention of the coder. We want to check here if we are in debug mode or not. I believe the best way to do it is to check the value of SF_DEBUG constant, instead of checking if some instance has a getLastExecutedQuery() method.

RuntimeException is a much better exception class here because it's not a normal error, which can be handled gracefully by the application. It's a programmer's error - it means that the programmer left some debug statements in a production code.

Please check attached patch for my solution.

06/03/08 12:39:01 changed by w.piekutowski

  • attachment patch.diff added.

Improve checking if debug mode is enabled and exceptions usage

07/02/08 15:17:13 changed by francois

(In [10067]) changed Exception class (refs #3529)

07/02/08 15:18:30 changed by francois

  • status changed from new to closed.
  • resolution set to fixed.

I changed the exception class but not the test - it seems to me that the query loggying should be controlled in one single location.

Feel free to reopen the ticket if you disagree.