Open weierophinney opened 4 years ago
@weierophinney can you look at this issue? It's looks seriously
Originally posted by @snapshotpl at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-64075777
I totally agree that ZF Api Problem should not interpret exception codes as http status codes. Exception codes can mean anything. For example, \PDOException uses code to transport the SQLSTATE error code.
I therefore propose that we extend ProblemExceptionInterface with a new method getHttpStatusCode() which returns an integer value to use. Otherwise we just assume 500.
@weierophinney could you please give feedback? If you want, i could create a pull request.
Originally posted by @BreiteSeite at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-72643061
@BreiteSeite makes sense; feel free to start a PR! :)
Originally posted by @weierophinney at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-75451818
How is the status of this issue or it's pull request? I think it would be very helpful, if the api-problem does not interpret the exception code as http status code.
Originally posted by @FrankGiesecke at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-85067956
I think api-problem should interpret the exception code only if the exception implements the ProblemExceptionInterface
(like the api-problem DomainException
).
Furthermore, IMHO, only codes in 4xx and 5xx ranges should be considered "problems". For example:
throw new \Exception('Error', 302);
will produce a 301 Moved Permanently
http status code, but without the Location
header.
I don't think it makes sense.
For this reason, I had to patch temporary some projects. Currently, I attached a listener to intercept exceptions:
use Zend\Mvc\MvcEvent;
use ZF\ApiProblem\Exception\ProblemExceptionInterface;
class UnhandledExceptionListener
{
public function __invoke(MvcEvent $e)
{
if (!$e->isError()) {
return;
}
$exception = $e->getParam('exception');
if ($exception
&& !$exception instanceof ProblemExceptionInterface
&& $exception instanceof \Exception
&& ($exception->getCode() < 400 || $exception->getCode() > 599)
) {
$internalServerErrorEx = new InternalServerErrorException($exception);
$e->setParam('exception', $internalServerErrorEx);
}
}
}
class InternalServerErrorException extends \Exception
{
public function __construct($previous)
{
parent::__construct('Internal Server Error', 500, $previous);
}
}
A possibile solution to avoid BCs in 1.0.x is to check the code ranges only. The $exception instanceof ProblemExceptionInterface
could be introduced later.
@weierophinney could you give me a feedback about above solutions, please? Then I could create a PR. I think it's a very important issue.
Originally posted by @leogr at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-89440992
@weierophinney any news?
Originally posted by @leogr at https://github.com/zfcampus/zf-api-problem/issues/27#issuecomment-185132111
When I throw exception in resource:
I get:
Apigility documentation says (https://apigility.org/documentation/api-primer/error-reporting):
And API problem documentation (https://tools.ietf.org/html/draft-nottingham-http-problem-06):
BTW Response after
looks funny :-)
Originally posted by @snapshotpl at https://github.com/zfcampus/zf-api-problem/issues/27