tan-tan-kanarek / github-php-client

MIT License
184 stars 120 forks source link

404 on issues that don't exist #54

Closed flashadvocate closed 9 years ago

flashadvocate commented 9 years ago

When an issue cannot be found, there's currently no recourse other than a mismatched response. (404 instead of 200).

tan-tan-kanarek commented 9 years ago

Please send your code so I'll understand what you're trying to accomplish, mostly in terms of what you expected to happen and what actually happened.

flashadvocate commented 9 years ago

https://github.com/tan-tan-kanarek/github-php-client/blob/master/client/GitHubClientBase.php#l384

When querying for an issue that doesn't exist:

ex:

$git->issues->getIssue(self::$owner, self::$repo, $erroneousId);

where id is an issue id that is erroneous, usually because it was typed in, an exception is thrown, rather than returning the response. It would be nice to be able to handle this error, rather than forcing an exception.

ex:

$issue = $git->issues->getIssue(self::$owner, self::$repo, $erroneousId);
if ($issue) {
    // do work
} else {
    // show a 404 error
}

Instead, you get:

$issue = $git->issues->getIssue(self::$owner, self::$repo, $erroneousId);
// thrown exception. you shall not pass 
tan-tan-kanarek commented 9 years ago

It's easy, just catch the exception.

try{
  $issue = $git->issues->getIssue(self::$owner, self::$repo, $erroneousId);
} catch(GitHubClientException $e){
  $errorDescription = $e->getMessage();
}

I decided to throw exception according to the returned HTTP error codes, assuming that if github team decided it should be considered as error than I should too.

Another approach would be to list all issues (listAllIssues) using some filter and check the number of returned results.

T.

flashadvocate commented 9 years ago

Fair enough. Thanks :)

On Fri, Jun 5, 2015 at 2:16 AM, Tan-Tan notifications@github.com wrote:

It's easy, just catch the exception.

try{ $issue = $git->issues->getIssue(self::$owner, self::$repo, $erroneousId);} catch(GitHubClientException $e){ $errorDescription = $e->getMessage();}

I decided to throw exception according to the returned HTTP error codes, assuming that if github team decided it should be considered as error than I should too.

Another approach would be to list all issues (listAllIssues) using some filter and check the number of returned results.

T.

— Reply to this email directly or view it on GitHub https://github.com/tan-tan-kanarek/github-php-client/issues/54#issuecomment-109173332 .