Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 2.5.0b5

Bug #12924 MDB2 fails unless error handling is set to PEAR_ERROR_RETURN
Submitted: 2008-01-17 20:31 UTC
From: haggholm Assigned: quipo
Status: Closed Package: MDB2 (version 2.4.1)
PHP Version: 5.2.1 OS: All
Roadmaps: (Not assigned)    
Subscription  


 [2008-01-17 20:31 UTC] haggholm (Petter Häggholm)
Description: ------------ The MDB2 codebase tacitly assumes, in many places, that the error handling style is set to PEAR_ERROR_RETURN, and that any failing call will return an error object. However, this is never checked, let alone enforced. Our project uses a custom error handler, which means that error objects are never returned. (It would of course fail in a similar manner with exception-based error handling, etc.) The solution, it appears to me, would be to surround critical sections -- that is, sections that are EXPECTED to return error objects -- with calls to pushErrorHandling(PEAR_ERROR_RETURN)/popErrorHandling(). As an example, a section from the MS-SQL module, mssql.php: function getTypeDeclaration($field) { $db->pushErrorHandling(PEAR_ERROR_RETURN); // !!! $db =& $this->getDBInstance(); $db->popErrorHandling(); // !!! if (PEAR::isError($db)) { return $db; } // ... } I am writing this up very quickly, but if not this exact code, then something extremely similar to this code should fix the problem. This would need to be done in many places in the MDB2 code (my text search suggests ~360).

Comments

 [2008-01-18 07:54 UTC] berdir (Sascha Grossenbacher)
How does your custom error handler looks like? PEAR::raiseError does _always_ return the error object, all other ERROR types are additional... actually even DIE (but it dies inside the contructor of pear_error, so it does not get so far). So, it is still possible (and needed!) to check if a pear_error was returned. The only "problem" should be, that the internal mdb2 error shows up in your custom error handler, but isn't this what you want?
 [2008-01-18 08:28 UTC] haggholm (Petter Häggholm)
Sorry, I seem to have left out an important piece of the puzzle in my initial report! What you say is true -- our custom error handler does catch all MDB2 errors, and normally this is what we want (well, not what I want, but what the people who wrote the initial codebase wanted and we have to live with for now...). I don't have the code at my fingertips right now, but basically it's the handler that gets called where nothing better has been put in place -- it's an error we don't know how to recover from, so print an error message and die. (This was written before the age of exceptions and we don't have the resources to refactor all the old code...) The problem is that within MDB2, there are certain places where errors are EXPECTED, and gracefully handled. An example (the one that slapped us rudely in the face) is the MS-SQL nextID() method, which attempts to insert some data into a possibly non-existent table, catches the (expected) error when no such table exists, creates it, and does its thing...except of course that our error handler has caught the error and terminated the application. In a sense, the problem is that MDB2 uses error objects to denote situations that aren't really errors. Any error that the MDB2 functions RETURN to our calling code is fine. The problem is with errors being used as internal signals, by MDB2. Of course it would be pointless to trap errors that are to be returned in any case, but when MDB2 relies on error objects as internal communication between its own methods, and doesn't bother to make sure that it's the right kind of communication, then there is a problem.
 [2008-01-21 22:08 UTC] quipo (Lorenzo Alberton)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.