"throws" section is not intuitive

[SerVB]

Continuing #17.

I've just tried it (using 0d4f589ea49e91f98f8c981d4c011f43578b7796 revision) and I've come up with the following:

Normal version

```kotlin data class SuccessResult(val ok: Boolean) { companion object { val NOT_OK = SuccessResult(ok = false) val OK = SuccessResult(ok = true) } } data class ProductUsable(val name: String, val id: Int, val type: Int) fun NormalOpenAPIRoute.addRoutes() { route("product") { post( info( summary = "Create a product.", description = "The product is created only if a product with the same ID does not exist. Returns `${SuccessResult::class.simpleName}` saying whether the product has been created." ), exampleResponse = SuccessResult.OK, exampleRequest = exampleProductUsable, body = { _, body -> createProduct(body, this::respond) } ) // ... } } ```

"Throws" version

```kotlin data class SuccessResult(val ok: Boolean) { companion object { val NOT_OK = SuccessResult(ok = false) val OK = SuccessResult(ok = true) } } data class ProductUsable(val name: String, val id: Int, val type: Int) private fun > T.throwsSuccessResultNotOk(fn: T.() -> Unit) { throws( APIException.apiException( status = HttpStatusCode.BadRequest, example = SuccessResult.NOT_OK, gen = { _: Throwable -> SuccessResult.NOT_OK } ), fn = fn ) } fun NormalOpenAPIRoute.addRoutes() { route("product") { throwsSuccessResultNotOk { post( info( summary = "Create a product.", description = "The product is created only if a product with the same ID does not exist. Returns `${SuccessResult::class.simpleName}` saying whether the product has been created." ), exampleResponse = SuccessResult.OK, exampleRequest = exampleProductUsable, body = { _, body -> createProduct(body, this::respond) } ) // ... } } } ```

The usability problems I see:

1. The generated exceptional response example doesn't match my NOT_OK (which is false):

See the image


2. When the request is missing some primitive fields, zeros are inserted. Here are some examples:

   1. Request: ProductUsable(name = "a", id = 5, type = 42)
      Expected response: 200 OK SuccessResult.OK Actual behavior in normal version: as expected Actual behavior in "throws" version: as expected
   2. Request: ProductUsable(name = "a", type = 42)
      Expected response: 400 Bad Request SuccessResult.NOT_OK (because id is missing) Actual behavior in normal version: 500 internal server error Actual behavior in "throws" version: 200 OK SuccessResult.OK (transformed as ProductUsable(name = "a", id = 0, type = 42))
   3. Request: ProductUsable(id = 5, type = 42)
      Expected response: 400 Bad Request SuccessResult.NOT_OK Actual behavior in normal version: 500 internal server error Actual behavior in "throws" version: as expected (maybe because a string is not a primitive type)

3. "Gen" function looks duplicating logic of the "example".

4. I don't understand which exception I should specify in "gen" function.

Finally, I propose a bit different design. It's inspired by exception block and I think it solves the 3 and the 4:

// inspired by:
//exception<JsonMappingException, Error>(HttpStatusCode.BadRequest) {
//    it.printStackTrace()
//    Error("mapping.json", it.localizedMessage)
//}

fun NormalOpenAPIRoute.addRoutes() {
    route("product") {
        catches<JsonMappingException, SuccessResult>(  // means that when JsonMappingException happens (can't generate the request body object), "generator" is called
            status = HttpStatusCode.BadRequest,
            example = SuccessResult.NOT_OK,
            generator = null  // means that the example is returned
        ) {
            post<Unit, SuccessResult, ProductUsable>(
                info(
                    summary = "Create a product.",
                    description = "The product is created only if a product with the same ID does not exist. Returns `${SuccessResult::class.simpleName}` saying whether the product has been created."
                ),
                exampleResponse = SuccessResult.OK,
                exampleRequest = exampleProductUsable,
                body = { _, body -> createProduct(body, this::respond) }
            )

            // ...
        }
    }
}

[Wicpar]

• Agreed that example should be returned if the generator is not specified, it should have been like this from the start. No need to create a different catch block.
• I do not understand what you mean with 1. , is the generated model using the wrong example?
• The gen function is there to allow for a more complex transform of the exception, In our production code we handle all errors with an error structure so that we can transmit the proper i18n codes for the error and additional metadata.
• in 4. , the exception you specify is the one you want to catch and handle. You can specify a supertype. For example an Exception will catch all exceptions, whereas JsonMappingException will only catch JsonMappingException

In the latest commit of the reworked-model branch:

throws(HttpStatusCode.BadRequest, CustomException::class) { ... // no example, just the exception handling
throws(HttpStatusCode.BadRequest, "example", CustomException::class) { ... // exception  handling with example, will respond example
throws(HttpStatusCode.BadRequest, "example", {ex: CustomException -> ex.toString()}) { ... // exception handling, will respond generated content

This should reflect the entirety of what you wanted if i understood correctly. Feel free to reopen the issue if not.

[SerVB]

Thanks, will answer tomorrow!

Feel free to reopen the issue if not.

Actually, "reopen" button seems to be invisible for me. Maybe a status like "Collaborator" is required. So I will just write here the results 😉

[Wicpar]

Alright, i'll reopen it if necessary

[SerVB]

Checked aeacf5158cea61773b898386fdbae0b17031bf69. It's much better!

I do not understand what you mean with 1. , is the generated model using the wrong example?

Yeah, the example is not that I've specified.

2.

So why are bad requests silently made good ones?

[Wicpar]

Oh right... That is jackson doing its magic with non nullable (primitive) fields. You can specify that it should fail with a jackson flag i think.

I need to see the full code to really help you there. Only request parameters are serialised by this library.

[SerVB]

My code is available here: https://github.com/SerVB/e-shop/tree/task-01.

That is jackson doing its magic with non nullable (primitive) fields.

But it doesn't do that magic without throws block: it fails on incomplete requests.

[Wicpar]

I just tested with your code, the throw clause really doesn't change parsing behavior... Default jackson configuration really does parse null and missing values to 0 on ints.

[SerVB]

I just tested with your code, the throw clause really doesn't change parsing behavior...

Hmm, can reproduce 500 error anymore... It really does insert 0 instead of absent key-values. Sorry...

However, there is one more issue mentioned:

I do not understand what you mean with 1. , is the generated model using the wrong example?

Yes, it's wrong:

It can be reproduced here: https://github.com/SerVB/e-shop/tree/a71b923d270c500ed68a6c9663f51a4b5af608bc . For a bad POST request, the response body should be {"ok": false}.

Could you investigate?

[Wicpar]

I indeed messed up, i missed a processing step that aggregated the information of multiple throw clauses: fixed in 0.2-beta.1-experimental