Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Result.map {} can cause a ClassCastException if an exception that isn't of the right type is thrown inside it #59

Closed
Benjozork opened this issue Sep 30, 2019 · 4 comments

Comments

@Benjozork
Copy link

It would be useful to add a function, let's say for example mapSafe that could handle an exception of another type, for example:

suspend fun <V : Any, U : Any, E : Exception, D : Exception> SuspendableResult<V, E>.mapSafe(transform: suspend (V) -> U): SuspendableResult<U, D> = try {
    when (this) {
        is SuspendableResult.Success -> SuspendableResult.Success(transform(value))
        is SuspendableResult.Failure -> SuspendableResult.Failure(error)
    }
} catch (ex: Exception) {
    SuspendableResult.error(ex as D)
}

Haven't tested this. Thoughts ?

@krzykrucz
Copy link

This happens in case of Result as well. It would be better not to handle these exceptions at all. Because ClassCastException in the least expected moment at runtime is not a good thing, in my opinion.

What do you think @kittinunf ?

@kittinunf
Copy link
Owner

To be honest, I am reluctant to add this functionality. Do you think you can have a top-level exception that every other error conforms too and use that one instead?

@R4md4c
Copy link

R4md4c commented Jan 6, 2021

Hope PR #74 gets merged and release soon as we've been suffering from this crash in production.

I think it's more reproducible when dealing with coroutines where sometimes a CancellationException is thrown due to the CoroutineScope of the current suspend method gets cancelled, and since in the mapError we're expecting our custom domain exception class it crashes instead due to ClassCastException.

Example:

remoteDataSource.retrieveFromDataSource() // Returns a SuspendableResult<Data, DataSourceException> 
    .mapError { exception: DataSourceException  ->   } // Crashes here because a JobCancellationException was thrown but we're expecting a DataSourceException

@kittinunf
Copy link
Owner

It has been merged quite some time ago! Please report it back whether it solves the problem or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants