Page 1 of 2

@ error suppressor

Posted: Mon Mar 15, 2010 11:24 am
by lshaw
I was wondering if it is considered bad practice to use the error supressor before checking for errors like:

Code: Select all

 
$conn=@mysql_connect("localhost","user","pass");
if(!$conn) {
    //trigger_error()
}
//or alteritivly
if(mysql_error()) {
    //trigger_error()
}
 
I saw a book the other day that was always using this method of handling errors when I have been told before never to use this?

Should you just use it for debbuggin puposes and turn errors off when the site is online?

thanks,

Lewis

Re: @ error suppressor

Posted: Mon Mar 15, 2010 12:11 pm
by AbraCadaver
lshaw wrote:Should you just use it for debbuggin puposes and turn errors off when the site is online?
Don't use @. Turn error reporting on site wide for development, turn them off for production.

Re: @ error suppressor

Posted: Tue Mar 16, 2010 12:17 am
by PHPHorizons
Concerning AbraCadaver's "don't", I can't see why it's a bad thing to suppress the error there. A live website shouldn't turn off error reporting in my opinion. There should be a custom error handler that logs errors. I can show a generic "An error occurred. We apologize any inconvenience." type message to the user.

Consider that if the database connection cannot be established, the user is going to be missing out on a lot of functionality of the page. I am assuming the db connection is there for a good reason. So it stands to reason that the user will be affected by it not being there. And the user should be told a problem has happened.

Suppressing the error will prevent that though and I don't personally suppress errors on my web sites (except when using serialize() and unserialize() where the errors are trivial and expected at times).

So here's the deal, if you are logging errors, suppressing the error will hide the connection error from your logs. If you aren't logging them, and you aren't displaying them, then you might as well suppress the error.

Re: @ error suppressor

Posted: Tue Mar 16, 2010 1:07 am
by John Cartwright
PHPHorizons wrote:Concerning AbraCadaver's "don't", I can't see why it's a bad thing to suppress the error there. A live website shouldn't turn off error reporting in my opinion. There should be a custom error handler that logs errors. I can show a generic "An error occurred. We apologize any inconvenience." type message to the user.
display_errors does not prevent the error from getting logged, nor does it prevent the usage from custom error handlers. By using error suppression, you are eliminating the possibility to generate notices/errors with display_errors on (which you generally want the highest error reporting during development).

Re: @ error suppressor

Posted: Tue Mar 16, 2010 9:58 am
by PHPHorizons
John Cartwright, there is certainly a lot of intricacies to error handlers that we can get into. And I'm well aware that using the error suppression operator has the effect of setting the error number to 0. The error handler will be sent the error, that is true. So in order to design an error handler that correctly handles error suppression, one simply ignores errors with an error number of 0. Doing that does prevent logging and displaying of the error. But one could design an error handler that doesn't take into account how error suppression works.

My comments from my previous post are true and geared towards how an error handler should correctly be designed. Simply because someone can design the error handler to do something different does not make my comments invalid.

Cheers

Re: @ error suppressor

Posted: Tue Mar 16, 2010 11:27 am
by alex.barylski
Error surpression is a hack, period. Any time you use it it's a sign of ignorance or laziness. Sometimes quick hacks make sense, but in the case above, it's a bad practice.

Use error surpression when your fixing some crappy script that throws errors because the server settings changed and u just want it to work. Otherwise, it's a horrible practice.

Cheers,
Alex

Re: @ error suppressor

Posted: Tue Mar 16, 2010 4:37 pm
by PHPHorizons
It's not a hack. It is one of the basic operators in the language. So long as it's use is understood by the programmer and the programmer decides it's helpful to the program, then it's a good thing to do.

I don't use it much at all. unserialize() is the only time I use it since that function emits errors when when the string is not unserializable. Perhaps there is a way to test for that, but I don't know it. Since I have switched over to using json instead of serializing, I haven't had a need to use error suppression. The intolerance PCSpectra shows, reminiscent of a Spanish Inquisition, goes far and beyond simply recommending against using error suppression (which in the vast majority of cases, I do recommend doing) and is definitely not in the spirit of this forum as I read it:
patrikG wrote:To help people learn how to program PHP
PCSpectra wrote:Any time you use it it's a sign of ignorance or laziness.
I see a fair bit of intolerance here and quite frankly, it's one reason I'm not terribly active in this forum.
Cheers

Re: @ error suppressor

Posted: Tue Mar 16, 2010 5:17 pm
by Eran
If you're using error suppression, that generally means you are not testing for data validity. You can only suppress errors below fatal errors, and those usually involve incorrect data/parameters passed on to functions. You would always want to know about those so you could check to see why they are being thrown - otherwise you could be missing possible bugs in your system.

I can't think of a single good use-case for using error-suppressing, except the one Alex has mentioned (having to support older / deprecated scripts).

Re: @ error suppressor

Posted: Tue Mar 16, 2010 6:02 pm
by Benjamin
This forum has a very diverse member base consisting of many different skill levels, intelligence levels and personality types from all over the world. When you ask a question or make a comment, you'll get many different answers from various members who are all looking at a particular issue from different angles. I don't feel that judging the entire forum based on the answers from any individual member is logical. Further, I believe that these characteristics are what make devnetwork the great place it is. We all accept each other here, we are all friends, but we certainly say what is on our mind.

Although the terms "ignorance" and "laziness" are ambiguous, I do believe this is a valid argument because in almost all cases a superior method of error suppression can be used in place of @. In all of the code I have ever written, I have maybe used it 2 or 3 times. There can be serious side effects when using @ for error suppression. This includes severe performance degradation, propagation, (using @include 'file.php'; will suppress errors for the entire file and every file it includes as well), very unpleasant debugging experiences and a few things I'm sure I am forgetting. I do believe that with this knowledge, continuing to use @ for error suppression would indeed be signs of ignorance and/or laziness.

In regards to the original post, there is no need to use @. Here is an example of how it could be done:

Code: Select all

 
    public function create($host, $user, $pass, $dbas, $port) {
        $this->host = $host;
        $this->user = $user;
        $this->pass = $pass;
        $this->dbas = $dbas;
        $this->port = $port;
 
        try {
            switch (MYSQL_DBTYPE) {
                case 'MYSQLI':
                    $this->dbClass = $this->mysqli();
                    break;
                case 'MYSQL':
                default:
                    $this->dbClass = $this->mysql();
            }
 
        } catch (Exception $e) {
            trigger_error($e->getMessage(), E_USER_WARNING);
        }
 
        return is_object($this->dbClass) ? $this->dbClass : false;
    }
 
    private function mysql() {
        if (!is_resource(($this->dbcon = mysql_connect("{$this->host}:{$this->port}", $this->user, $this->pass)))) {
            throw new Exception("Failed to create MySQL database connection.  #" . mysql_errno() . ' - ' . mysql_error());
        }
 
        if (!mysql_select_db($this->dbas, $this->dbcon)) {
            throw new Exception("Failed to create MySQL database connection.  #" . mysql_errno() . ' - ' . mysql_error());
        }
 
        return new dbmysql($this->dbcon);
    }
 
    private function mysqli() {
        if (!is_object(($this->dbcon = mysqli_connect($this->host, $this->user, $this->pass, $this->dbas, $this->port)))) {
            throw new Exception("Failed to create MySQLi database connection.  Error: " . mysql_errno() . ' - ' . mysql_error());
        }
 
        return new dbmysqli($this->dbcon);
    }
 
When executing a query, the following method for error detection could be used:

Code: Select all

 
    public function query($query)
    {
        $result = (!mysqli_multi_query($this->link_id, $query)) ? false : true;
 
        if (!$result) {
            $this->result = false;
            trigger_error("Error: " . mysqli_errno($this->link_id) . ' ' . mysqli_error($this->link_id), E_USER_WARNING);
        } else {
            $this->store_result();
        }
 
        return $result;
    }
 

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:09 pm
by Weirdan
astions wrote:

Code: Select all

 
if (!is_resource(($this->dbcon = mysql_connect("{$this->host}:{$this->port}", $this->user, $this->pass)))) {
            throw new Exception("Failed to create MySQL database connection.  #" . mysql_errno() . ' - ' . mysql_error());
}
Unfortunately it will still emit warning, even though you're handling failure yourself (by throwing exception). See for yourself:
[text] weirdan@virtual-debian:/home/sam$ php -r 'error_reporting(E_ALL|E_STRICT); if(!is_resource(mysql_connect("qwe", "qwe", "qwe"))) throw new Exception("oops");' Warning: mysql_connect(): Unknown MySQL server host 'qwe' (1) in Command line code on line 1 Fatal error: Uncaught exception 'Exception' with message 'oops' in Command line code:1Stack trace:#0 {main}  thrown in Command line code on line 1 [/text]

IMO if exceptional situation is handled by the userland (php script) it should not cause any logging. It's easily achievable with newer APIs that could use exceptions - but older APIs like procedural mysql_* do not behave in this (sane) way.

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:16 pm
by Eran
I think that's an excellent example of a warning you always want to know about and should never suppress.

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:20 pm
by Benjamin
Weirdan wrote:Unfortunately it will still emit warning, even though you're handling failure yourself (by throwing exception).
Yes you are correct. I neglected to mention that I was using a customized error handler. A good solution to this problem is to set a dummy error handler before calling mysql_query. Then restore the original error handler after calling it. Although this seems to be more trouble than it's worth, it can in fact improve performance in many cases.

Here is some code for a custom error handler.

Code: Select all

 
function my_custom_error_handler($level, $message, $file, $line) {
    switch ($level) {
        case 2:    $level = 'E_WARNING';           break;
        case 8:    $level = 'E_NOTICE';            break;
        case 256:  $level = 'E_USER_ERROR';        break;
        case 512:  $level = 'E_USER_WARNING';      break;
        case 1024: $level = 'E_USER_NOTICE';       break;
        case 6143: $level = 'E_ALL';               break;
        case 2048: $level = 'E_STRICT';            break;
        case 4096: $level = 'E_RECOVERABLE_ERROR'; break;
        default:   $level = "UNKNOWN ERROR $level";
    }
 
    $message = "$message in \"$file\" line $line";
 
    $system_logger->add_event('function: my_custom_error_handler', $file, $line, $message, $level);
    
    if (!IN_PRODUCTION_MODE)
    {
        echo '<div style="font-family: sans-serif; font-size: 12px; border-width: 1px; border-style: solid; border-color: #ff0000; background-color: #ffffff; color: #000000; padding: 3px; margin: 3px;">' . $message . '</div>';
    }
    
    return true;
}
 
You could also route errors to a dummy error handler:

Code: Select all

 
function null_routing_error_handler($level, $message, $file, $line) {
    return true;
}
 

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:35 pm
by alex.barylski
It's not a hack. It is one of the basic operators in the language. So long as it's use is understood by the programmer and the programmer decides it's helpful to the program, then it's a good thing to do.
I disagree. It's a valid operator, sure, but that doesn't make it any less of a bad practice. Even language designers make mistakes. :P
The intolerance PCSpectra shows, reminiscent of a Spanish Inquisition, goes far and beyond simply recommending against using error suppression (which in the vast majority of cases, I do recommend doing) and is definitely not in the spirit of this forum as I read it:
Intolerance? It's a bad practice in almost all cases (99%). As for your example of unserialize(). If that occurs, I would consider/look for a flaw in the design of the system, maybe??? Why would the string you pass to unserialize() ever be non-unserializable??? You should be testing for that condition, like if the string is NULL/EMPTY don't bother unserializing in the first place.

I have never had a need to use @ operator, only temptation because the alternative meant extra work.

Cheers,
Alex

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:35 pm
by Darhazer
If you suppress error/warning of mysql_connect and the mysql extension is not enabled, you'll get a fatal error (call to undefined function) which results in a blank screen. Error suppressing of fatal errors is really bad idea ;) Once we've lost about a hour finding why our product do not work on a particular hosting, and the reason was error suppressing of mysql_connect and mysql_connect was undefined function. given the fact everybody is switching to mysqli, there's a change for anybody to get a host without old mysql functions :)

Re: @ error suppressor

Posted: Tue Mar 16, 2010 7:41 pm
by Benjamin
Yeah, that's pretty much what I meant when I said, "very unpleasant debugging experiences".