Continue throwing same exception when edit entity

[abozhinov]

Now I have event listener and throw exception, but the exception is stopped by the controller and replace with BadRequestHttpException.

[javiereguiluz]

Thanks for this proposal ... but I'm not sure if we can merge this "as is" without breaking things for other people 🤔

[abozhinov]

In both cases you trigger Exception. BadRequestHttpException is used only in the edit form. Also everyone read the change log before update.

[ndench]

I agree with @javiereguiluz, this would a BC break, so at the very least it'll need to go into 5.x. I also agree that people should be reading the change log, but semantic versioning exists so that you only have to read the change log for major version updates. No one is reading the change log for every minor version up for every package they depend on.

However, I'm not sure what value it provides. @abozhinov can you please provide some more details and maybe an example of what you're trying to acomplish?

I think, given that this is a controller, it shouldn't be throwing random exceptions. It's meant to throw HttpExceptions so that the correct http status code is returned.

If you're trying to catch specific exceptions, could you instead override this method in your CrudController? for example:

public function edit(AdminContext $context)
{
    try {
        return parent::edit($context);
    } catch (BadRequestHttpException $e) {
        // some custom logic
    }

    if ($event->isPropagationStopped()) {
        return $event->getResponse();
    }

    return new Response($newValue ? '1' : '0');
}

[abozhinov]

Hi,
I think this change is very important because:

1. Have Page entity.
2. In EventListener you add some logic and throw Exception with specific Message that you want to show.
3. When edit Page Entity you don't receive the Exception you throw in the EventListener instead that you receive BadRequestHttpException with empty message.

If you think this change can break something let's set Message to BadRequestHttpException?

[ndench]

Thanks for the extra detail. I think this is a much better solution!

[javiereguiluz]

Things are more clear to me now ... and the updated code looks safer to merge 🙌 Thanks Alexandar!