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

Throw a HttpException when the request is sucessfull but the http status are not 2XX, like Retrofit #457

Open
DevSrSouza opened this issue Oct 10, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@DevSrSouza
Copy link

DevSrSouza commented Oct 10, 2023

Is your feature request related to a problem? Please describe.
Using Retrofit, by default, when using suspend, when an API returns successfully (the backend actual responds), when the status code is not success (2XX) it throws a custom HttpException.

When working with Ktorfit and Ktor, when using Suspend and the direct type of the Response Body, when the server returns a 4XX with a Custom Body or Any Body at all, the Exception is ContentConvertException, this does not say nothing of what was the really cause of the error.

Describe the solution you'd like
Creating a custom HttpException and throwing the Exception when the response is not 2XX like Retrofit does:
https://github.com/square/retrofit/blob/8abb8af6ce730b137c2a9f95c3bb5164c93e955e/retrofit-adapters/scala/src/main/java/retrofit2/adapter/scala/BodyCallAdapter.java#L47-L51

Describe alternatives you've considered
There are workarounds that I found that is not the best but is working so far for me that is creating a custom Converter.SuspendResponseConverter<HttpResponse, Any> that overrides the default IMPL of Ktorfit.

ktorfit {
    ....
    converterFactories(UnsuccessResponseConverterFactory())
}

public class KtorfitHttpException(
    @Transient public val response: HttpResponse,
    public val bodyText: String,
) : RuntimeException() {
    override val message: String
        get() = "HTTP " + response.status.value + " " + bodyText
}

internal class UnsuccessResponseConverterFactory : Converter.Factory {

   class UnsuccessResponseSuspendConverter(
        val typeData: TypeData,
        val ktorfit: Ktorfit
    ) : Converter.SuspendResponseConverter<HttpResponse, Any> {
        override suspend fun convert(response: HttpResponse): Any {
            return try {
                if (response.status.isSuccess()) {
                    response.call.body(typeData.typeInfo)
                } else {
                    throw KtorfitHttpException(response, response.bodyAsText())
                }
            } catch (exception: Exception) {
                throw exception
            }
        }
    }

    override fun suspendResponseConverter(
        typeData: TypeData,
        ktorfit: Ktorfit
    ): Converter.SuspendResponseConverter<HttpResponse, *>? {
        if (typeData.typeInfo.type != Response::class) {
            return UnsuccessResponseSuspendConverter(typeData, ktorfit)
        }
        return null
    }
}
@DevSrSouza DevSrSouza added the enhancement New feature or request label Oct 10, 2023
@DevSrSouza DevSrSouza changed the title Throw a HttpException when the request is sucessfull but the http status are not, like Retrofit Throw a HttpException when the request is sucessfull but the http status are not 2XX, like Retrofit Oct 10, 2023
@deividasstr
Copy link

Unfortunately, with version 2, this does not work - argument HttpResponse is replaced with KtorfitResult, where only success case has the notion of HttpResponse, but not failure.

Any workarounds for v2?

@deividasstr
Copy link

@Foso , any suggestions how to tackle this with v2?

@Foso
Copy link
Owner

Foso commented Aug 28, 2024

@deividasstr isn't that working?

internal class UnsuccessResponseConverterFactory : Converter.Factory {
    class UnsuccessResponseSuspendConverter(
        val typeData: TypeData,
        val ktorfit: Ktorfit
    ) : Converter.SuspendResponseConverter<HttpResponse, Any?> {
        override suspend fun convert(result: KtorfitResult): Any =
            when (result) {
                is KtorfitResult.Failure -> {
                    throw result.throwable
                }

                is KtorfitResult.Success -> {
                    if (result.response.status.isSuccess()) {
                        result.response.call.body(typeData.typeInfo)
                    } else {
                        throw KtorfitHttpException(result.response, result.response.bodyAsText())
                    }
                }
            }
    }

    override fun suspendResponseConverter(
        typeData: TypeData,
        ktorfit: Ktorfit
    ): Converter.SuspendResponseConverter<HttpResponse, Any?>? {
        if (typeData.typeInfo.type != Response::class) {
            return UnsuccessResponseSuspendConverter(typeData, ktorfit)
        }
        return null
    }
}

@deividasstr
Copy link

@Foso thanks, this should work. I assumed that is KtorfitResult.Failure is for every non 200 response.

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

No branches or pull requests

3 participants