Interoperability issue with 500 response code caused by ApplicationException about winter

[LukeTowers]

It's not Winter's responsibility to care about what proxies are doing to the response. In the provided example of a repeater's minimum items not being reached then yes, that would probably be more correct to be handled with a ValidationException instead, but that's up to the developer writing the logic to decide which exception is most relevant.

Do you have a suggestion for more relevant status codes / exceptions to use instead of 500 & an ApplicationException? The ApplicationException is intended for when the application cannot proceed with processing, but it's not because of a coding error or something that should be fixed by a developer.

[lex0r]

It's not Winter's responsibility to care about what proxies are doing to the response.

An application doesn't operate in a vacuum, there are 3rd parties and they all rely on some common standards that Web has to offer. When an app doesn't adhere to them for a good reason then it's hard to blame it, however when there's no good reason to give a 500 response for an error which is not fatal, then this behaviour should be changed. It's not only about the environment, it's about providing a logically correct behaviour.

but that's up to the developer writing the logic to decide which exception is most relevant.

I would like to know what led to that decision. On the surface it looks like the developer took an opportunity to come up with a quick error handling mechanism which is already there, and just inherited the specifics of it. But this issue is raised exactly for this reason - many developers may want to use it so more pressure is put on making it right and compatible.

Do you have a suggestion for more relevant status codes / exceptions to use instead of 500 & an ApplicationException? The ApplicationException is intended for when the application cannot proceed with processing, but it's not because of a coding error or something that should be fixed by a developer.

Right, this class of errors in HTTP terms fall into 4xx category - client error (as opposed to 5xx when server fails to fulfil an apparently valid [i.e. inputs are correct and match the context] request).

I did some custom implementation by redefining ErrorHandler via Event::listen('exception.beforeRender') and used HTTP code 400. To make it fully compatible with ApplicationException I also changed the detailed error message to just the message used in the exception constructor. Can share some snippets if that helps...

[LukeTowers]

I wouldn't be opposed to a PR to specify the HTTP error code to be used along with providing the most common error codes as constants on the class because sometimes a 422 is more relevant.

however when there's no good reason to give a 500 response for an error which is not fatal, then this behaviour should be changed. It's not only about the environment, it's about providing a logically correct behaviour.

I would argue that an ApplicationException is fatal. Unlike a stateful server application, the ApplicationException might not indicate that things are now permanently broken, but for the purposes of the request being made when the ApplicationException is thrown the code is saying "I've reached a state where I cannot help you anymore; here's why".

What changes would you like to propose? I'm not opposed to an audit of the existing uses of ApplicationException vs SystemException in the core and improving our documentation / developer education resources to better communicate when each exception type should be used, we just need to figure out what changes specifically we want to make and then make them :)