kovenant icon indicating copy to clipboard operation
kovenant copied to clipboard

Rethrow exception in fail block

Open friendoye opened this issue 7 years ago • 5 comments

I would like to catch any Exception in fail block and then rethrow it as WrappedException. Code should be look like that:

task {
    ...
} fail { error ->
    throw WrappedException(message = "Something went wrong", cause = error)
}

But instead of providing WrappedException in sequential fail blocks, kovenant provides the same error. As I can see that's happening because lambdas, that are passed to fail, are used as callbacks instead of something like map function. What are you thoughts on this problem and what can you suggest to overcome this problem?

friendoye avatar Dec 19 '17 23:12 friendoye

The fail callback does not create a new Promise. So any then function operates on the original promise. What you're probably looking for is function that's more common in the javascript community: then(success: (V1) -> V2, failure: (E1) -> E2) : Promise<V2, E2>

So a single method that both transforms the success and failure values and returns a new Promise.

mplatvoet avatar Dec 20 '17 20:12 mplatvoet

I think its better to add then(success: (V1) -> V2, failure: (E1) -> E2) : Promise<V2, E2> and then(failure: (E1) -> E2) : Promise<V, E2> to the library (the last one function probably could have a better name). Could you add this functions to kovenant? Or you're open for PR? 🤔

friendoye avatar Dec 20 '17 20:12 friendoye

I'm not convinced it should be added to the library. What are the arguments for adding this?

Though it can easily be added to peoples own projects:

fun <V1, V2> Promise<V1, Exception>.then(context: Context = this.context,
                                         s: (V1) -> V2,
                                         f: (Exception) -> Exception) : Promise<V2, Exception> {
    val deferred = deferred<V2, Exception>(context)
    success {
        try {
            deferred.resolve(s(it))
        } catch (e: Exception) {
            deferred.reject(e)
        }
    }

    fail {
        try {
            deferred.reject(f(it))
        } catch (e: Exception) {
            deferred.reject(e)
        }
    }

    return deferred.promise
}

fun main(args: Array<String>) {
    task<String> {
        throw Exception("one")
    }.then(s = {s-> s}) {
        throw Exception("${it.message} and two")
    } fail {
        println(it.message)
    }
}

mplatvoet avatar Dec 20 '17 21:12 mplatvoet

In Android world it is pretty common scenario, where you want to map some 3rd party library Exception (for example, FacebookLoginFailedException) to application defined exception (LoginFailedException), so you can isolate implementation details. Without function, that was mentioned earlier, you'll ended up using deffered or try-catch block in every place, where Exception mapping is happening. I might write an extension for Promise or some kind of wrapper, that will be doing this work for me, but it seems more convenient to add then(success: (V1) -> V2, failure: (E1) -> E2) : Promise<V2, E2>. P.S. I'm not from JavaScript world, but in my opinion usage of this function across JavaScript developers is pretty common.

friendoye avatar Dec 20 '17 22:12 friendoye

This is a huge limitation. All promise/task libraries we've worked with in the past allow manipulating the promise chain so that you can map errors (for example the Bolts library's continueWith(..) VS continueWithTask(..) – reference here https://github.com/BoltsFramework/Bolts-Android). This allows you to log error messages in your application services / business layer for example, and present any user UI in your presentation layer (Activities/Fragments). Mapping from 3rd-party exception types to custom exceptions as mentioned by @friendoye is also a core use case.

This essentially means you cannot do any custom logic along the failure chain, only the success one (ie: with 'then').

Please, please, add these in. The library is otherwise so great, but cannot be deemed mature without such core functionality missing.

Here's a real-world example:

Presentation layer:

UserService.resetPassword(this, username)
    .successUi {
        Toast.makeText(this, getString(R.string.reset_password_success), Toast.LENGTH_SHORT).show()
        finish()
    }.failUi { x ->
        progress_bar.visibility = View.GONE
        Toast.makeText(this, x.message, Toast.LENGTH_SHORT).show()
    }

Application/business layer:

object UserService {
    ...

    fun resetPassword(context:Context, email: String): Promise<Unit, Exception> {
        return FirebaseAuthServices.resetPassword(email)
            .then(mapFailure = {
                Exception(context.getString(R.string.error_forgot_password))
            })
    }
}

NOTE: Examples above use a convenience version of your method above which only has the failure mapping code included – since this can be a common scenario. ie:

// (!) Kovenant is currently missing this functionality. Tracked feature request here: https://github.com/mplatvoet/kovenant/issues/9
fun <V1> Promise<V1, Exception>.then(context: Context = this.context,
    mapFailure: (Exception) -> Exception
) : Promise<V1, Exception> {
    return then(context, { value -> value }, mapFailure)
}

fun <V1, V2> Promise<V1, Exception>.then(context: Context = this.context, mapSuccess: (V1) -> V2/* = { value -> value }*/,
    mapFailure: (Exception) -> Exception
) : Promise<V2, Exception> {
    val deferred = deferred<V2, Exception>(context)
    success {
        try {
            val value = mapSuccess(it)
            deferred.resolve(value)
        } catch (e: Exception) {
            deferred.reject(e)
        }
    }

    fail {
        try {
            val exception = mapFailure(it)
            deferred.reject(exception)
        } catch (e: Exception) {
            deferred.reject(e)
        }
    }

    return deferred.promise
}

marchy avatar Nov 24 '18 21:11 marchy