Unify error responses

[pdambrauskas]

Had to replace custom response building logics from session status validation with thrown exception, to route it through the same flow as other request errors are routed.

in case of not found error:

{
  "message": "Not Found",
  "path": "/lighter/api/sessions/123",
  "errors": [
    {
      "message": "Page Not Found"
    }
  ]
}

In case of invalid session state:

{
  "message": "Bad Request",
  "path": "/lighter/api/sessions/829b6551-a7d9-4b9f-a774-bd21d13f3f97/statements",
  "errors": [
    {
      "message": "Invalid session state: ERROR",
      "details": {
        "sessionState": "error"
      }
    }
  ]
}

in case of validation error:

{
  "message": "Bad Request",
  "path": "/lighter/api/sessions/829b6551-a7d9-4b9f-a774-bd21d13f3f97/statements",
  "errors": [
    {
      "message": "statement.code: must not be null"
    }
  ]
}

@Minutis
/cc @jmilkiewicz

[jmilkiewicz]

IMHO it is a horrible approach... Probably using exception for flow control is a standard practice in micronaut but it does not change it is a horrible: an exception is thrown somewhere, no matter how deep it is thrown and then there is a magic ExceptionHandler which will produce HttpResponse. It is just long-jump, very similar to goto

[pdambrauskas]

IMHO it is a horrible approach... Probably using exception for flow control is a standard practice in micronaut but it does not change it is a horrible: an exception is thrown somewhere, no matter how deep it is thrown and then there is a magic ExceptionHandler which will produce HttpResponse. It is just long-jump, very similar to goto

This is how it works on Micronaut, as well as on Spring Boot. It allows framework users to focus more on business logic and not to focus on error handling and other supporting features.

IMHO In this specific case it is ok to throw an exception from the Service layer, since the client of the service is trying to use it in unexpected manner.

[jmilkiewicz]

I know that both springBoot and micronaut allows one to create exceptionHandlers . If you start to model any alternative output from function/method as exception the mechanism can look pretty tempting.
Nevertheless it drives to "goto/long-jumps" code, using 'goto' and 'long-jumps' seems tempting but as the code grow it becomes pain in ass: an exception somewhere, it can also be like 3-4 layers below and you are catching sth on top... This kind of approach just brings more harm than benefit.

The concept of exception handlers is ok, if your outer layer catches true exceptions but the cases like: session in invalid state or validation error are not exceptions - they are just a regular, easy-to-predict cases, not sth like connection got broken, DB is down or whatever...

IMHO In this specific case it is ok to throw an exception from the Service layer, since the client of the service is trying to use it in unexpected manner.

Fully disagree.
How client of service layer does know that session is in "invalid state"? It does not know,

Of course you can build protocol that forces one to first "check session state" before submitting a statement but how would you enforce this protocol. What would happen if in a time gap between checking if session is in valid state and statement submission, session become invalid ?

[pdambrauskas]

I know that both springBoot and micronaut allows one to create exceptionHandlers . If you start to model any alternative output from function/method as exception the mechanism can look pretty tempting.

Not only that - It has many handlers pre-built. For example Spring Boot has a handler for http client exceptions, Validation exceptions (hibernate validator, micronaut validator, other validators throws exception if request payload is not valid), etc...
Due to this, you have to go against the framework, if you want to go wit a different approach & have unified error response body logics.

While your comments regarding Exception usage for flow control makes sense, despite I do not fully agree with it in this specific case, I do not want to have multiple converged flows for error handling:

• Payload Validation (micronaut validator) response building is based on Exception handler
• Optional response body handler is also built into framework and returns 404 (probably also through some kind of exception)
• I do not have to introduce yet another path for 404, through a custom mapper
• I do not want to introduce another response body construction path for 400 response through custom mappers

So either we need to disable these framework features, and go with the new Result - Response mapping approach or go with the framework.

[jmilkiewicz]

The fact that spring boot or micronaut does something it does not mean ( out-of-box ) it is a valid solution...
If you like to have a long-jumps, go for it.

Again, if we were in kotlin/scala/typescript/haskell a standard way of returning multiple possible outputs from method (in this case a service) it by using algebraic data types - as I described in #710

My point is that by using "these weird mappers" you have full (and local control) how to map service output to httpresponse and everything is very explicit. if it happens that there is a lot of commonality between mappers you can solve it using regular software engineering practices.

Payload Validation (micronaut validator) response building is based on Exception handler

Input Payload Validation is (at least in spring boot) handled as exception, but one does not need to do it that way. BTW do you think validation is part of a logic or not? If it is a part of logic than it shall be part of service, not part of controller...

Optional response body handler is also built into framework and returns 404 (probably also through some kind of exception)

Optional is not handled by exception, it is first-class citizen class and it is treated appropriately not like runtime exception,

I do not have to introduce yet another path for 404, through a custom mapper
I do not want to introduce another response body construction path for 400 response through custom mappers

Look how inconsistent it is: some HTTP responses are defined via return types, like using Optional and some by exception which are can be thrown multiple layers inside.

TLDR: if you look long-jumps and control flow by exceptions, go for it