[Question] Error handling about laravel-responder HOT 15 CLOSED

Hey Evgenii :)

Have you copy pasted all the lines from the render method of the package's exception handler? Including:

        if ($exception instanceof ApiException) {
            return $this->renderApiError($exception);
        }

from laravel-responder.

Hi @Flugger,

Thanks for the reply!
Yes I did, and this is my full method code:

    public function render($request, Exception $exception)
    {
        // Uncomment to debug travis-ci output
        // throw $exception;

        $this->transformException($exception);

        if ($exception instanceof ApiException) {
            return $this->renderApiError($exception);
        }

        return parent::render($request, $exception);
    }

from laravel-responder.

Hmm, that's weird. I assume you've imported the full Flugg\Responder\Exceptions\Http\ApiException file in the top?

What errors are you getting if you enable debug mode?

from laravel-responder.

@Flugger, my intent is to get rid of the Woops page and make sure all the responses either success or error return in JSON format.

For example, I don't have a default route, here is the message – http://realty-staging-api.herokuapp.com/

Another example, with an error – http://realty-staging-api.herokuapp.com/offers?filters[type]=50

from laravel-responder.

Yes, correct. Thats what the code above should fix. Perhaps Lumen throws a different exception on page not found than Laravel. I'll do some investigation and get back to you!

from laravel-responder.

In the second example, my application throws a standard UnexpectedValueException. Maybe I should re-catch in the handler?

from laravel-responder.

If your UnexpectedValueException extends ApiException it should automatically be caught with the code in your handler. I just created a Lumen application and did some testing and realised, there is no default exception transformer for page not found errors. I definitely think it should be included out of the box and will look into getting it into the v2 release. As a workaround, you can catch the 404 errors manually in the render method:

use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

if ($exception instanceof NotFoundHttpException) {
      return responder()->error('page_not_found', 404, 'The page you are looking for could not be found.');
}

from laravel-responder.

Did you find a solution?

I also just realised UnexpectedValueException is a standard PHP exception, so that would also need to be caught and handled, like the NotFoundHttpException above. Generally speaking, it might be a good idea to make exceptions that extend the abstract class ApiException and throw them instead. That way you will get the automatic converting to error responses without having to intercept them yourself.

from laravel-responder.

Hi @Flugger,

or maybe make a mapping like so:

$mapping = [
    UnexpectedValueException::class => NotFoundHttpException::class
];

from laravel-responder.

Yeah, that's definitely a good idea and something I've considered in the past, but stuff like the ValidationFailedException expects a validator argument which is hard to pull off with the array version.

from laravel-responder.

Hmm, what about allowing closures, so you can do this:

$mappings = [
    UnexpectedValueException::class => NotFoundHttpException::class,
    ValidationException::class => function ($exception) {
        return new ValidationFailedException($exception->validator);
    },
];

Edit: Just realised this would have to be done in a method as you can't put expressions into default properties.

from laravel-responder.

I've been playing around with it some more and so far this will be possible in v2:

    public function render($request, Exception $exception)
    {
        $this->convert($exception, [
            UnexpectedValueException::class, PageNotFoundException::class,
            ValidationException::class, function($exception) {
                throw new ValidationFailedException($exception->validator);
            }
        ]);

        $this->convertLaravelException($exception);

        if ($exception instanceof ApiException) {
            return $this->renderErrorResponse($exception);
        }

        return parent::render($request, $exception);
    }

Edit: updated the code

from laravel-responder.

Hi @Flugger,

That looks good to me, I thought about my problem as well, probably the best thing to do in my case is to wrap the functionality with try ... catch.

try {
    if (some critical stuff)
        throw new UnexpectedValueException;
} catch(UnexpectedValueException $e) {
    return responder()->error('oops_1');
} catch(ValidationException $e) {
    return responder()->error('oops_2');
}

The thing is, some of these exceptions could be thrown by 3rd party extensions that I installed via Composer, so I cannot update them and extend from ApiException.

from laravel-responder.

Yeah, thats usually how I solve it in my apps :) You can of course also catch the exceptions and rethrow your own versions that extend ApiException.

from laravel-responder.

Alright, thanks a lot! I'm closing this for now ...

from laravel-responder.