Comments

Added third option to Application::$catchExceptions#63

Open

enumag wants to merge 1 commit intonette:master from

enumag:exceptions

Open

Added third option to Application::$catchExceptions #63
enumag wants to merge 1 commit intonette:master from
enumag:exceptions

Conversation

@enumag

Copy link

Contributor

@enumag enumag commented Feb 15, 2015

I use a bunch of services to check whether the application is in state to run the request, if the use has privileges and so on. However these services are not supposed to handle the error correctly with a flashmessage and redirect, they only throw an exception (some specific descendant of BadRequestException).

I handle these exceptions in ErrorPresenter which works fine but I have some troubles with this in debug mode - I don't want to see these exceptions because they are kind of expected and I want to see if the ErrorPresenter handles them correctly. Therefore in debug mode I want to catch BadRequestException.

I consider this approach to be far better then the usual. When writing the application I don't repeat the flash messages and redirects everywhere (it get's even worse if I need to translate them - I need to inject the translator). Instead I just throw an exception and later add a line or two to ErrorPresenter. Presenters and controls are much cleaner because they focus on what they are supposed to do and don't have to deal with errors at all.

What do you think about this solution? I use it for about year or so and am really satisfied with it.

@enumag


 Added third option to Application::$catchExceptions

248ecd0

@enumag

Copy link

Contributor Author

enumag commented Feb 15, 2015

It gets even better when you use module-specific ErrorPresenters like I do. I'll try to send a PR with that later if this is merged.

@fprochazka

Copy link

Contributor

fprochazka commented Feb 15, 2015

I'm not sure I understand. But I like the idea, that the BadRequestException won't show up in tracy but rather in ErrorPresenter even in debug mode.

@JanTvrdik

Copy link

Contributor

JanTvrdik commented Feb 15, 2015

What about this solution to your problem:

Enable catching all exceptions regardless of debugMode
In ErrorPresenter check if debugMode is enabled or not and then either pass them directly to Tracy error handler or handle in ErrorPresenter as usual.

@JanTvrdik

Copy link

Contributor

JanTvrdik commented Feb 15, 2015

By looking at the code I think that another solution may be possible – implement onError event listener in which you either enable or disable catchExceptions based on catched exception and debugMode.

@enumag

Copy link

Contributor Author

enumag commented Feb 15, 2015

@JanTvrdik You missed the purpose of this PR entirely. I'm not looking for ways to do what the PR does without changing Nette, I already know several ways to do that without your help. I want to know if you like the idea of putting error handling in ErrorPresenter. If so I'd like to make it the recommended way for Nette, hence this PR to make it convenient for other people than myself.

@JanTvrdik

Copy link

Contributor

JanTvrdik commented Feb 15, 2015

Ok, regarding this PR. I hate the magic NULL value. But the idea of having a way to pass BadRequestException to error presenter in dev mode is good.

@enumag

Copy link

Contributor Author

enumag commented Feb 15, 2015

Good. :-) I'm not really happy with the NULL value either, I just couldn't think of a better solution - none of these seem much better to me:

Pass string 'smart' instead.
Use some constants like CATCH_ALL, CATCH_NONE, CATCH_SMART. Values could be bools/ints/strings whatever (probably bools for compatibility + a string or null for CATCH_SMART).
Pass a type to catch \Exception would catch all, NULL would catch none, \Nette\Application\BadRequestException for the desired behaviour.

Any suggestions?