https://kotlinlang.org logo
#http4k
Title
# http4k
d

dave

04/06/2018, 7:30 PM
@elifarley Thanks for the PR - I've managed to fix it in a more generic way - you can now optionally provide a function when creating the
CatchLensFailure
to create a custom response. New method looks like this:
Copy code
ServerFilters.CatchLensFailure { lensFailure -> Response(OK).body(lensFailure.localizedMessage) }
. Releasing in 3.23.0
e

elifarley

04/13/2018, 3:16 AM
@dave, I've left a new comment on https://github.com/http4k/http4k/issues/116
your solution is indeed generic (so let's keep it), but it is reached too late in the chain of events, where the originating exception information is long gone (sorry that I could only test it today)
My PR #117 should work as it is located at the very beginning of the chain, where the originating exception is still known.
Albeit not a generic solution
d

dave

04/14/2018, 10:14 AM
Just investigated this and TBH I'm confused. I've created this Gist which shows the stack trace with and without your PR. You can see that the information you wanted to surface is available, albeit in the "cause" field of the LensFailure.. https://gist.github.com/daviddenton/154aab8d3391ec7faaf1c8d0ffc62159
on the same note if you use Jackson, you get much better errors - there's an example in the Gist. Jackson also handles nullable fields in a better way since it has the Kotlin module installed
e

elifarley

04/14/2018, 10:55 AM
Yep, the cause field is where the information lies. But if you look at the screenshot on the Github issue #116, you'll see that the initial LensFailure that contains the non-null cause field is gone. Instead, we have another LensFailure created later on, which does not contain anything on the cause field.
Looks like the initial information-rich LansFailure is transformed into a bare-bones LensFailure. I forgot the name of the class and line number where this transformation happens.
Regarding Jackson - What if the LensFailure has nothing to do with JSON ?
d

dave

04/14/2018, 10:59 AM
then you'll still have the same information.
what I'm trying to say is that the current code does contain all of the information that you wanted, albeit in the cause of the lensfailure rather than the main message
e

elifarley

04/14/2018, 10:59 AM
yep, but Jackson won't help there, right?
d

dave

04/14/2018, 11:00 AM
let me do another example...
e

elifarley

04/14/2018, 11:00 AM
Doesn't seem to be the case in my investigations - the original LensFailure object is discarded, and another one is created, this time with empty _ cause_
I can redo the debugging session and take note of the place where the new LensFailure is created and the original one discarded
d

dave

04/14/2018, 11:01 AM
Can you give me a gist which recreates it?
here's that example anyway:
Copy code
Query.zonedDateTime().required("foo")(Request(GET, "/?foo=asd"))
gives:
Copy code
Exception in thread "main" org.http4k.lens.LensFailure: query 'foo' must be string
	at org.http4k.lens.Lens.invoke(lens.kt:17)
	at org.http4k.testing.LensKt.main(lens.kt:11)
Caused by: java.time.format.DateTimeParseException: Text 'asd' could not be parsed at index 0
	at java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:1949)
	at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1851)
	at java.time.ZonedDateTime.parse(ZonedDateTime.java:597)
	at org.http4k.lens.LensSpecKt$zonedDateTime$1.invoke(lensSpec.kt:209)
	at org.http4k.lens.LensSpecKt$zonedDateTime$1.invoke(lensSpec.kt)
	at org.http4k.lens.LensGet$map$1.invoke(lensSpec.kt:227)
	at org.http4k.lens.LensGet$map$1.invoke(lensSpec.kt:16)
	at org.http4k.lens.LensGet$invoke$1.invoke(lensSpec.kt:17)
	at org.http4k.lens.LensGet$invoke$1.invoke(lensSpec.kt:16)
	at org.http4k.lens.BiDiLensSpec$required$1.invoke(lensSpec.kt:164)
	at org.http4k.lens.Lens.invoke(lens.kt:13)
	... 1 more
e

elifarley

04/14/2018, 11:02 AM
Yep, I'll do it
d

dave

04/14/2018, 11:02 AM
ta
are you using the contracts module?
e

elifarley

04/14/2018, 11:05 AM
yep
d

dave

04/14/2018, 11:06 AM
aha - ok. that's probably why you're not seeing the cause. we combine all the errors into a single failure:
Copy code
override fun invoke(nextHandler: HttpHandler): HttpHandler =
        { req ->
            val body = routeMeta.body?.let { listOf(it::invoke) } ?: emptyList<(Request) -> Any?>()
            val errors = body.plus(routeMeta.requestParams).fold(emptyList<Failure>()) { memo, next ->
                try {
                    next(req)
                    memo
                } catch (e: LensFailure) {
                    memo.plus(e.failures)
                }
            }
            if (errors.isEmpty()) nextHandler(req) else throw LensFailure(errors)
        }
but your PR won't fix that 🙂
e

elifarley

04/14/2018, 11:06 AM
That's the place I saw!
this is where the cause is lost
Is this a lost cause?
😉
it won't fix that. However, the cause's message is captured and sent forward withing the LensFailure message, so in a way , this cause is not lost.
it ends up at the other end of the tunnel
d

dave

04/14/2018, 11:09 AM
I'll have a think about how to fix it best.
e

elifarley

04/14/2018, 11:09 AM
hmm good!
more generic solutions are always welcome!
How about changing the routeMeta to contain a function that could be used to extract the Exception's message and append it to the memo?
d

dave

04/14/2018, 11:11 AM
the reason I'm reticent to mash the cause together by default is that we already use mash them together in the CatchLensFailure. they get appended to the status of the response:
Copy code
Response(BAD_REQUEST.description(it.failures.joinToString("; "))
e

elifarley

04/14/2018, 11:12 AM
I see
d

dave

04/14/2018, 11:12 AM
whatever we come up with will have to work with the non-contract version as well.
e

elifarley

04/14/2018, 11:13 AM
What's the name of the file that contains the code you've just shown me?
d

dave

04/14/2018, 11:13 AM
ContractRouteSpec
e

elifarley

04/14/2018, 11:13 AM
thanks
d

dave

04/14/2018, 11:13 AM
(that's the class, not the file
e

elifarley

04/14/2018, 11:16 AM
Is the class _ RouteMeta_ a good place to have a custom function to help us here?
d

dave

04/14/2018, 11:19 AM
this version will capture the first cause...
Copy code
abstract class ContractRouteSpec internal constructor(val pathFn: (PathSegments) -> PathSegments,
                                                      val routeMeta: RouteMeta,
                                                      vararg val pathLenses: PathLens<*>) : Filter {
    abstract fun with(new: RouteMeta): ContractRouteSpec
    abstract infix operator fun <T> div(next: PathLens<T>): ContractRouteSpec

    open infix operator fun div(next: String) = div(Path.fixed(next))

    override fun invoke(nextHandler: HttpHandler): HttpHandler =
        { req ->
            val body = routeMeta.body?.let { listOf(it::invoke) } ?: emptyList<(Request) -> Any?>()
            val overall = body.plus(routeMeta.requestParams).fold(null as LensFailure?) { memo, next ->
                try {
                    next(req)
                    memo
                } catch (e: LensFailure) {
                    memo?.let { LensFailure(it.failures + e.failures, e) } ?: e
                }
            }
            overall?.let { throw it } ?: nextHandler(req)
        }

    internal fun describe(contractRoot: PathSegments): String = "${pathFn(contractRoot)}${if (pathLenses.isNotEmpty()) "/${pathLenses.joinToString("/")}" else ""}"
}
(which is at least better)
e

elifarley

04/14/2018, 11:22 AM
it is
d

dave

04/14/2018, 11:23 AM
I could possibly expand each "failure" to include the cause...
then only use the first part by default in the message..
e

elifarley

04/14/2018, 11:24 AM
and how to deviate from default?
d

dave

04/14/2018, 11:25 AM
in the custom CatchLensFailure function
(which we already have)
e

elifarley

04/14/2018, 11:25 AM
Dave, I'll have to go, my 5yo will have a Judo presentation. Thanks for helping me with this!
d

dave

04/14/2018, 11:26 AM
np. I'll keep looking at it for a bit. enjoy the judo!
e

elifarley

04/14/2018, 11:26 AM
thanks 🙂
d

dave

04/15/2018, 11:09 AM
On reflection, I think the existing change is enough, as the body is always the first contract parameter to be processed and hence you'll get the cause correctly populated. Other parameter types (query headers etc) have a name already so they are easier to debug. I'll release a new version with this change in asap.
it's going out in v3.23.1
3 Views