https://kotlinlang.org logo
Channels
Powered by
i

itnoles

02/25/2019, 3:58 AM
@Derk-Jan Karrenbeld Could we reduce a number of "cause by" exceptions?
d

Derk-Jan Karrenbeld

02/25/2019, 1:26 PM
Probably, what's the use-case? In general we really do want to follow throws, wraps and rethrows.
i

itnoles

02/25/2019, 7:42 PM
it generated a lot of noise on the logcat.
d

Derk-Jan Karrenbeld

02/25/2019, 7:43 PM
You generally shouldn't log these exceptions?
i

itnoles

02/25/2019, 7:45 PM
I think Android Log is stripped on the release builds.
d

Derk-Jan Karrenbeld

02/25/2019, 7:46 PM
It is
i

itnoles

02/25/2019, 7:56 PM
By looking to https://github.com/kittinunf/fuel/issues/601#issuecomment-467056798, it looks same strack trace called it twice.
d

Derk-Jan Karrenbeld

02/25/2019, 7:58 PM
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

itnoles

02/25/2019, 8:01 PM
d

Derk-Jan Karrenbeld

02/25/2019, 8:02 PM
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

itnoles

02/25/2019, 8:03 PM
if it is already FuelError, why needs to do Bubble it?
d

Derk-Jan Karrenbeld

02/25/2019, 8:03 PM
Because otherwise the stacktrace is replaced.
And then you don't know where the original error comes from.
i

itnoles

02/25/2019, 8:04 PM
Coroutines already have Throwable Functions for it.
d

Derk-Jan Karrenbeld

02/25/2019, 8:04 PM
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

itnoles

02/25/2019, 8:06 PM
com.github.kittinunf.fuel.core.requests.SuspendableRequest.prepareResponse(SuspendableRequest.kt:32) com.github.kittinunf.fuel.core.requests.SuspendableRequest.awaitResult(SuspendableRequest.kt:43)
d

Derk-Jan Karrenbeld

02/25/2019, 8:06 PM
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

itnoles

02/25/2019, 8:09 PM
it does stink
d

Derk-Jan Karrenbeld

02/25/2019, 8:09 PM
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

itnoles

02/25/2019, 8:12 PM
I have to look it later.
✔️ 1
breakpoint would help 🙂
d

Derk-Jan Karrenbeld

02/25/2019, 8:13 PM
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

itnoles

02/25/2019, 8:15 PM
executeRequest already have getOrThrow
d

Derk-Jan Karrenbeld

02/25/2019, 8:20 PM
Yes and?
i

itnoles

02/26/2019, 3:22 AM
I did breakpoint, it does hit throw FuelError.wrap(error, pair.second)
👍 1
2 Views
Powered by