Added third option to Application::$catchExceptions

[enumag]

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]

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]

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]

What about this solution to your problem:

1. Enable catching all exceptions regardless of debugMode
2. 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]

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]

@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]

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]

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:

1. Pass string 'smart' instead.
2. 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).
3. Pass a type to catch \Exception would catch all, NULL would catch none, \Nette\Application\BadRequestException for the desired behaviour.

Any suggestions?

[EdaCZ]

It would be nice to have clean solution for this problem. Sometimes I deal with it. I would appreciate this "feature" :+1:

[enumag]

@JanTvrdik What do you think about this? https://github.com/enumag/application-1/commit/bef93921042ca85191e2ec1efd1ef97e66ca5077

[enumag]

ping @JanTvrdik

[JanTvrdik]

I need this today :smile: But I still don't like your proposal.

[enumag]

@JanTvrdik Any of them? :-D https://github.com/nette/application/pull/63#issuecomment-74442020 Not even this? https://github.com/enumag/application-1/commit/bef93921042ca85191e2ec1efd1ef97e66ca5077 Do you have some suggestions?

[JanTvrdik]

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.

[enumag]

Let me see... Nope, still don't like that. :-)

[f3l1x]

@enumag Nice. I'm looking forward to it.

[josefsabl]

Any progress on this?

https://forum.nette.org/en/31306-catchexceptionx-for-4xx-errors-only

[enumag]

@josefsabl Since my suggestions were refused I just kept using my own hacked Application class (contains some other hacks too) to do this. I'm not working with nette anymore so don't expect me to send any PRs regarding this. Though I'm still interested to see if some solution gets implemented in nette.

[josefsabl]

@josefsabl Since my suggestions were refused I just kept using my own hacked Application class (contains some other hacks too) to do this. I'm not working with nette anymore so don't expect me to send any PRs regarding this. Though I'm still interested to see if some solution gets implemented in nette.

What a shame. Thanks for the reply anyway.

[josefsabl]

I extended the \Nette\Application\Application class like this:

final class Application extends \Nette\Application\Application
{
    /**
     * @throws \Throwable
     */
    public function run(): void
    {
        try {
            parent::run();
        } catch (\Nette\Application\BadRequestException $e) {
            $this->processException($e);
        }
    }
}

and changed the application.application service to use this class instead like this in one of my extensions:

$this->getContainerBuilder()
    ->getDefinition('application.application');
    ->setFactory(\My\Application::class);

... any idea what might go wrong? :-)

Looks like it is working well after the quick test.