https://kotlinlang.org logo
Join Slack
Powered by
<@U0M0EAG90> Thanks for the PR - I've managed to f...
# http4k
d
@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
@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
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
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
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
yep, but Jackson won't help there, right?
d
let me do another example...
e
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
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
Yep, I'll do it
d
ta
are you using the contracts module?
e
yep
d
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
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
I'll have a think about how to fix it best.
e
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
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
I see
d
whatever we come up with will have to work with the non-contract version as well.
e
What's the name of the file that contains the code you've just shown me?
d
ContractRouteSpec
e
thanks
d
(that's the class, not the file
e
Is the class _ RouteMeta_ a good place to have a custom function to help us here?
d
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
it is
d
I could possibly expand each "failure" to include the cause...
then only use the first part by default in the message..
e
and how to deviate from default?
d
in the custom CatchLensFailure function
(which we already have)
e
Dave, I'll have to go, my 5yo will have a Judo presentation. Thanks for helping me with this!
d
np. I'll keep looking at it for a bit. enjoy the judo!
e
thanks 🙂
d
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
Open in Slack
PreviousNext
Powered by