Title
e

elifarley

03/09/2018, 10:37 AM
I have an HttpHandler that is returning
HTTP/1.1 400 body 'body' must be object
This message is hiding an exception. How can I get hold of it? I have these filters:
routes(
"/sgzr/api/v1" bind Api.router(productService, orderService)
).let {
DebuggingFilters.PrintRequestAndResponse()
                .then(ServerFilters.Cors(CorsPolicy.UnsafeGlobalPermissive))
                .then(ServerFilters.CatchAll())
                .then(ServerFilters.CatchLensFailure)
                .then(it)
}
I think it would be nice to have the exception message or even the stack trace included in the response body
And does the order of CatchAll and CatchLensFailure matter ?
d

dave

03/09/2018, 10:44 AM
the ordering does matter - the CatchAll will turn any uncaught exception into a 500. The CatchLensFailure catches LensFailures and turns them into a 400
the LensFailure DOES contain the exception that caused it, so if you write you own CatchLensFailure implementation you can do as you please for the moment:
object CatchLensFailure : Filter {
        override fun invoke(next: HttpHandler): HttpHandler = {
            try {
                next(it)
            } catch (lensFailure: LensFailure) {
                when {
                    lensFailure.target is Response -> throw lensFailure
                    lensFailure.target is RequestContext -> throw lensFailure
                    lensFailure.overall() == Failure.Type.Unsupported -> Response(UNSUPPORTED_MEDIA_TYPE)
                    else -> Response(BAD_REQUEST.description(lensFailure.failures.joinToString("; ")))
                }
            }
        }
    }
e

elifarley

03/09/2018, 11:00 AM
It turns out I only needed to invert the order of the 2 catchers, which now is:
.then(ServerFilters.CatchLensFailure)
.then(ServerFilters.CatchAll())
Before this, only
HTTP/1.1 400 body 'body' must be object
would appear By doing this, a nice stack trace appears on stdout (only
Here's what I get now:
HTTP/1.1 500 Internal Server Error

org.http4k.lens.LensFailure: body 'body' must be object
	at org.http4k.contract.ContractRouteSpec$invoke$1.invoke(routeSpec.kt:31)
	at org.http4k.contract.ContractRouteSpec$invoke$1.invoke(routeSpec.kt:11)
	at org.http4k.core.Http4kKt$then$2.invoke(Http4k.kt:19)
[,...]
But your suggestion seems better
The Lens Failure was caused by an
InvalidJSONException: Could not convert to a JSON Object or Array
. I want to have this information readily available. I'll follow your suggestion of creating a custom CatchLensFailure
d

dave

03/09/2018, 11:55 AM
yep - it's a general failure. are you using GSON?
e

elifarley

03/09/2018, 11:55 AM
yep
d

dave

03/09/2018, 11:56 AM
ok - currently we swallow the exception and it isn't passed to the InvalidJsonException. I'll change that to add the cause
(which might give you a better example of what's going on and why your JSON is invalid)
actually - it looks like that exception is directly caused by something in your code not being able to convert a string to an object or an array:
override fun String.asJsonObject(): JsonElement = JsonParser().parse(this).let { if(it.isJsonArray || it.isJsonObject) it else throw InvalidJsonException() }
e

elifarley

03/09/2018, 12:00 PM
that would be great, thanks!
in fact, the problem is that the reference is null
d

dave

03/09/2018, 12:01 PM
so there isn't currently a cause to display
e

elifarley

03/09/2018, 12:01 PM
But at least there's the name of the exception to be shown - InvalidJsonException
d

dave

03/09/2018, 12:01 PM
yeah - you can see that the parse doesn't fail - it fails the test in the let
e

elifarley

03/09/2018, 12:02 PM
Maybe
throw InvalidJsonException("Not an array nor an object: $it")
would it make sense?
d

dave

03/09/2018, 12:04 PM
yeah - I'll change it to be a message
e

elifarley

03/09/2018, 12:05 PM
Having more contextual information on the exception feels good.
Here's how I would change the LensFailure class:
open class LensFailure(val failures: List<Failure>, override val cause: Exception? = null, val target: Any? = null)
    : Exception(failures.joinToString { "$it" } + (cause?.let { " ($it)" } ?: ""), cause) {
Does it look good?
d

dave

03/09/2018, 12:20 PM
no need to change the LensFailure. we've already got the cause as a field if we need it - you can get the message from there.
Generally we use the message as a general purpose thing and then we can drill down to get the cause message if we need it
e

elifarley

03/09/2018, 12:21 PM
got it
d

dave

03/09/2018, 12:21 PM
so in your new CatchLensFailure filter, you can just access the cause and then add it if required
e

elifarley

03/09/2018, 12:21 PM
yep, exactly