Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handing of ValidationExceptions by exception handler #407

Open
oskar-koli opened this issue Oct 14, 2024 · 3 comments
Open

Handing of ValidationExceptions by exception handler #407

oskar-koli opened this issue Oct 14, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@oskar-koli
Copy link
Contributor

oskar-koli commented Oct 14, 2024

Version

Tested only on v4, but looking at the code it seems to be also present in v5.

What did you expect to happen?

When a ValidationException is thrown from a route, I would expect the request to be redirected to the previous page with an error bag or in the case of a JSON endpoint returning validation errors.

This behavior has been implemented in the Illuminate exception handler's convertValidationExceptionToResponse method. But Acorn's exception handler's render method overrides the base class' render method, which results in convertValidationExceptionToResponse never being called for ValidationExceptions.

Is there some reason for this simplified render method being used, instead of the full implementation which removes support for proper ValidationException handling? This could be fixed by either adding support for ValidationException to the render method override or otherwise maybe even just using the base method, if it doesn't cause any issues. Thoughts?

What actually happens?

ValidationExceptions are treated like any other exception and results in the generic error view being rendered.

Steps to reproduce

  1. Throw ValidationException in a route
  2. See that the exception does not result in a redirect or correct json response.

System info

No response

Log output

No response

Please confirm this isn't a support request.

Yes

@oskar-koli oskar-koli added the bug Something isn't working label Oct 14, 2024
@Log1x
Copy link
Member

Log1x commented Oct 14, 2024

Thanks for the detailed issue!

This is due to the exception handler stuff being implemented prior to any sort of routing.

Care to do a PR?

@oskar-koli
Copy link
Contributor Author

Ok cool, thanks for the quick response!
Do you think it should be doable just to use the base render method or do some obvious issues come to mind with that?
Will of course test it either way, but just in case you have any thoughts :)

I'll be happy to do a PR, will take a look at it in the coming days 👍

@Log1x
Copy link
Member

Log1x commented Oct 14, 2024

it looks like we'd need to bring in illuminate/auth at a glance if we wanted to support the base render method but that should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants