https://kotlinlang.org logo
Join Slack
Powered by
<@UDB0VFBPV> Could we reduce a number of "cause by...
# kotlin-fuel
i
@Derk-Jan Karrenbeld Could we reduce a number of "cause by" exceptions?
d
Probably, what's the use-case? In general we really do want to follow throws, wraps and rethrows.
i
it generated a lot of noise on the logcat.
d
You generally shouldn't log these exceptions?
i
I think Android Log is stripped on the release builds.
d
It is
i
By looking to https://github.com/kittinunf/fuel/issues/601#issuecomment-467056798, it looks same strack trace called it twice.
d
Actually seems to error on two locations: - 
SuspendableRequest.prepareResponse(SuspendableRequest.kt:34)
caused by - 
SuspendableRequest.prepareResponse(SuspendableRequest.kt:32)
Everything else is coroutine wrapper, not the same stack
i
d
What's odd?
The first time the error gets caught, the second time it gets wrapped (because it's an continuation -> rethrow, but already a fuel error)
i
if it is already FuelError, why needs to do Bubble it?
d
Because otherwise the stacktrace is replaced.
And then you don't know where the original error comes from.
i
Coroutines already have Throwable Functions for it.
d
In our code base we sometimes generate a fuel error, but then we pass it on and on, and then later that error gets unwrapped (from result) and therefore thrown again.
Yes, but we don't have 2 paths, one that throws one that doesn't. Instead, we ALWAYS throw, but sometimes capture it
i
com.github.kittinunf.fuel.core.requests.SuspendableRequest.prepareResponse(SuspendableRequest.kt:32) com.github.kittinunf.fuel.core.requests.SuspendableRequest.awaitResult(SuspendableRequest.kt:43)
d
The reason we need this is because: -> runCatching may catch a fuelerror -> mapCatching may throw a fuel error
Copy code
runCatching { prepareRequest(request) }
            .mapCatching { executeRequest(it) }
            .mapCatching { pair ->
                // Nested runCatching so response can be rebound
                runCatching { prepareResponse(pair) }
runCatching -> might throw mapCatching -> might throw mapCatching -> might throw runCatching -> might throw
We need this, because each step may throw, and each step may actually be deferred. Before we had a bug where errors would not be wrapped (remember? @Nikky). This makes sure we always wrap.
i
it does stink
d
In this case, there is a bug on line 40 + 41
It should NOT mapcatch, it should get or throw before it
But I don't know the correct fix right now -- need to write some tests
i
I have to look it later.
✔️ 1
breakpoint would help 🙂
d
When you try to reduce this, remember: - once you have an error, you need to wrap it in fuelerror or it blows up result - once you have a response, we want it in fuelerror, so we have fuelerror.response - once you have a fuelerror.response, you want to use that instead of a fake response
i
executeRequest already have getOrThrow
d
Yes and?
i
I did breakpoint, it does hit throw FuelError.wrap(error, pair.second)
👍 1
2 Views
Open in Slack
PreviousNext
Powered by