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

RFC: Always catch client exceptions #234

Open
mabar opened this issue Sep 22, 2019 · 0 comments
Open

RFC: Always catch client exceptions #234

mabar opened this issue Sep 22, 2019 · 0 comments

Comments

@mabar
Copy link
Contributor

mabar commented Sep 22, 2019

BadRequestException is thrown or $presenter->error() is called when we want terminate application because client sent an invalid request (e.g. for non-existing post). This kind of error is expected to happen and not needed to be rethrown and displayed by Tracy, as it's a client, not application error. Currently if we want test that client error is forwarded to error presenter and error presenter displays correct output (eg. Post not found or a generic Page not found) then catchExceptions needs to be enabled. Also, logs should be checked to ensure none real (server) error happened at the same time.

If we want to see BadRequestException, it can be handled in error presenter, but with most common workflow simplified.

Other usages of BadRequestException should be also considered.

  • ForbiddenRequestException - extends BadRequestException, is used to reject request with unsufficient permissions in Presenter::checkRequirements() - behavior can be changed same way as for errors thrown in presenters.
  • BadSignalException, Application::createInitialRequest(), Application::processRequest(), Component::tryCall() and other methods - most of these can be application errors or client errors at the same time - behavior should remain same, we want see these errors in Tracy. It's not needed to care about case when error is caused by user as it usually means user tries non-existing signals, invalid values etc.

Suggested solution: Split BadRequestException in two. One for real application errors with same behavior as current, second for client errors which would be always forwarded to error presenter. I am willing to send a PR.

I would also like to change ApplicationExtensions catchExceptions behavior. It should be imho used only if debugMode is enabled. For production mode should exceptions always be catched, independently of if catchExceptions is enabled, to prevent misconfiguration.
Or even better, add debugMode to Application and check it here.
- Solved by 1e117d3

Related PRs: #63, #173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant