Thread
#kotlin-fuel
    itnoles

    itnoles

    3 years ago
    @Derk-Jan Karrenbeld Could we reduce a number of "cause by" exceptions?
    d

    Derk-Jan Karrenbeld

    3 years ago
    Probably, what's the use-case? In general we really do want to follow throws, wraps and rethrows.
    itnoles

    itnoles

    3 years ago
    it generated a lot of noise on the logcat.
    d

    Derk-Jan Karrenbeld

    3 years ago
    You generally shouldn't log these exceptions?
    itnoles

    itnoles

    3 years ago
    I think Android Log is stripped on the release builds.
    d

    Derk-Jan Karrenbeld

    3 years ago
    It is
    itnoles

    itnoles

    3 years ago
    By looking to https://github.com/kittinunf/fuel/issues/601#issuecomment-467056798, it looks same strack trace called it twice.
    d

    Derk-Jan Karrenbeld

    3 years ago
    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
    d

    Derk-Jan Karrenbeld

    3 years ago
    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)
    itnoles

    itnoles

    3 years ago
    if it is already FuelError, why needs to do Bubble it?
    d

    Derk-Jan Karrenbeld

    3 years ago
    Because otherwise the stacktrace is replaced.
    And then you don't know where the original error comes from.
    itnoles

    itnoles

    3 years ago
    Coroutines already have Throwable Functions for it.
    d

    Derk-Jan Karrenbeld

    3 years ago
    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
    itnoles

    itnoles

    3 years ago
    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

    3 years ago
    The reason we need this is because: -> runCatching may catch a fuelerror -> mapCatching may throw a fuel error
    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.
    itnoles

    itnoles

    3 years ago
    it does stink
    d

    Derk-Jan Karrenbeld

    3 years ago
    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
    itnoles

    itnoles

    3 years ago
    I have to look it later.
    breakpoint would help 🙂
    d

    Derk-Jan Karrenbeld

    3 years ago
    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
    itnoles

    itnoles

    3 years ago
    executeRequest already have getOrThrow
    d

    Derk-Jan Karrenbeld

    3 years ago
    Yes and?
    itnoles

    itnoles

    3 years ago
    I did breakpoint, it does hit throw FuelError.wrap(error, pair.second)