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

Bug #7850 constructor doesn't return PEAR Error Object
Submitted: 2006-06-09 15:01 UTC
From: hkrems at yahoo dot de Assigned: till
Status: Closed Package: Mail_Queue (version 1.1.3)
PHP Version: 5.1.2 OS: Max OS X
Roadmaps: 1.2.3    
Subscription  


 [2006-06-09 15:01 UTC] hkrems at yahoo dot de (Harald Kremsner)
Description: ------------ creating an instance of Mail_Queue with invalid dsn did not return a PEAR Error. Test script: --------------- $db_options['type'] = 'db'; $db_options['dsn'] = 'mysql://user:pass@localhost/test'.'ERROR'; // << generate error $db_options['mail_table'] = 'mail_queue'; $mail_options['driver'] = 'sendmail'; $mail_options['host'] = 'localhost'; $mail_options['port'] = 25; $mail_options['auth'] = false; $mail_queue =& new Mail_Queue($db_options, $mail_options); if(PEAR::isError($mail_queue)) die ($mail_queue->getMessage()); Expected result: ---------------- final words: "Mail Queue Error: Cannot connect to database" Actual result: -------------- PEAR::isError($mail_queue) simply returned FALSE

Comments

 [2006-06-09 15:10 UTC] quipo (Lorenzo Alberton)
get the latest CVS version
 [2006-06-13 08:02 UTC] hkrems at yahoo dot de
I have tested it with the latest CVS versions (I think): Queue.php,v 1.16 Body.php,v 1.10 Container.php,v 1.9 db.php,v 1.20 the only way to check if there is an error is like if(PEAR::isError($mail_queue->container->db)) { print ('<h1>ERROR</h1>'); }
 [2006-06-13 10:10 UTC] quipo (Lorenzo Alberton)
sorry, you're right, the error object isn't propagated. you could replace ==================================== if (PEAR::isError($this->db)) { return new Mail_Queue_Error(MAILQUEUE_ERROR_CANNOT_CONNECT, $this->pearErrorMode, E_USER_ERROR, __FILE__, __LINE__, 'DB::connect failed: '. DB::errorMessage($this->db)); } ==================================== with ==================================== if (PEAR::isError($this->db)) { trigger_error(DB::errorMessage($this->db), E_USER_ERROR); } ==================================== in function Mail_Queue_Container_db($options) In my local copy I completely revised the error handling for the package, but it's not backward compatible, so I can't commit it. I hope I can find some time (or some external help) for this package...
 [2007-11-30 12:53 UTC] bate (Marco Kaiser)
i take over this bug
 [2008-04-30 15:20 UTC] till (Till Klampaeckel)
@Lorenzo I'd help with Mail_Queue2 :) (Hoping it's PHP5.) Aside from that, you cannot return anything from a constructor. Which is why a factory pattern is nice. For example: function factory($db, $mail) $cls = new Mail_Queue($db, $mail); if (PEAR::isError($cls->db)) { // or similar return $cls->db; } return $cls; }
 [2008-04-30 16:19 UTC] quipo (Lorenzo Alberton)
@Till: yes, I know you can't return errors from a constructor, that was a really bad design decision of an early version that went into the stable release and couldn't be changed afterwards. A factory would have a much wiser decision, but the hasty release cycle prevented a serious revision of the whole error handling management :-(
 [2008-05-05 08:14 UTC] till (Till Klampaeckel)
We could add a factory (along with the construct) and document that it's the suggested way, what do you think? I will check with the BC-elitists on IRC, if you think that's ok. I'll put it in. Luckily in PHP5, we won't have those issues.
 [2008-05-05 08:20 UTC] till (Till Klampaeckel)
Just a final comment. Not possible to fix this bug right now, we'll keep it open for Mail_Queue2.
 [2008-05-05 08:24 UTC] quipo (Lorenzo Alberton)
yes, I'd leave it open for Mail_Queue2. The whole error handling system of MQ1 needs a revision, the constructor is only part of the problem.
 [2008-05-15 12:37 UTC] till (Till Klampaeckel)
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. Ok, this bug is fixed without breaking BC. A new release coming shortly. If you can give us feedback already, you are more than welcome! We introduced a new Mail_Queue::factory() method, but you may also use Mail_Queue::hasErrors() and Mail_Queue::getErrors() to retrieve them directly.