3

What is the best way to return errors from a PHP function, when the function has executed normally?

Example

public function login($user, $pw){
     if(!$this->verifyUser($user))
         // return error about invalid user
     }
     else if (!$this->verifyPw($pw)){
        // return error about invalid pw
     }
     else {
         // return OK
     }
}

Caller - Return response as JSON for web UI

public function doLogin($user,$pw){
    $res = $this->login($user, $pw);
    return json_encode($res);
}

On one hand I could understand returning results as an array, but I feel like this does not make sense for such low level functions. Perhaps they should return error codes and then caller must lookup the error code string?

jozenbasin
  • 2,142
  • 2
  • 14
  • 22
  • What is the intended output usage case? API response or Javascript DOM response (Ajax)? Are you using jQuery? – Marc May 20 '17 at 22:58
  • Eventually the result bubbles up to a form response in HTML, which is determined by the JSON response. If the JSON response has a `status` of `0` there was an error, and a string `msg` would be attached. Otherwise the `status` is 1 and all is OK. – jozenbasin May 20 '17 at 23:05
  • Where I am spinning my wheels is - when is it OK to implement this error handling? Obviously only the `login` function knows exactly what went wrong, but should it return an entire error package (array) or just an error code? – jozenbasin May 20 '17 at 23:08
  • Well, in general to protect users from hackers trying to guess login details, you should be less specific about what exactly was wrong, else you clueing a hacker into what fields are incorrect. – Marc May 20 '17 at 23:15
  • This is just an example that I made up. – jozenbasin May 20 '17 at 23:16

2 Answers2

3

Assuming you are in an object, you basically have three major options:

  • store errors in something like $this->errors array and return false
  • have some kind of error-collector as a dependency for object, where you
    call $this->collector->addError('blah blah'); and return false
  • throw an exception

For the first two approaches, you will have to check the return value, and based on that, pull the list of errors. But both of those options have the benefit of being able to collect multiple errors.

The exception approach is a bit lighter on coupling, but you can only get one error.

As for what to actually return, I would recommend going with error code + description string. But that string would not be returned by your class. Instead your error should be registered using some "placeholder", that later is translated:

$this->errors[] = [
    'code' => 52, 
    'msg' => 'authentication.login.invalid-password',
];

When you pull the errors from your object, it would be basically a list of entries like this, And then you just run them through your translation service.

In a case of exception, that same information would reside in $e->getCode() and $e->getMessage(), when your object throws InvalidPassword exception.

tereško
  • 58,060
  • 25
  • 98
  • 150
  • I just find exceptions wrong. The function executed correctly, but the user supplied the wrong credentials (for example). I personally think exceptions should only be used when the application literally could not do what it was asked (because for example, a file was not found, a connection could not be made, etc). – jozenbasin May 20 '17 at 23:06
1

For an API response the answer from tereško would be along the correct lines.

For a DOM response you can do the following:

I have used a response code only in the past for something so simple:

http://php.net/manual/en/function.http-response-code.php with code 401

public function login($user, $pw) {

 header_remove(); # Clear all previous headers.

 if( !$this->verifyUser($user) || !$this->verifyPw($pw) ){
    http_response_code(401);
    exit;
 }

 http_response_code(200);
 exit;
}

jQuery:

$.ajax({
  .......
  statusCode: {
    200: function() {
      window.location.href = '/';
    },
    401: function() {
      alert( "Login Failed" );
    }
  }
});
Marc
  • 5,109
  • 2
  • 32
  • 41