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

Client doesn't throw for HTTP Errors when Any is the expected result. #1209

Open
ScottPierce opened this issue Jun 27, 2019 · 3 comments
Open
Assignees
Labels
ux User Experience issue

Comments

@ScottPierce
Copy link

Ktor Version

1.2.2

Ktor Engine Used(client or server and name)

Client w/ OkHTTP Engine on Android

JVM Version, Operating System and Relevant Context

IntelliJ IDEA 2019.1.3 (Ultimate Edition)
Build #IU-191.7479.19, built on May 28, 2019
JRE: 1.8.0_202-release-1483-b58 x86_64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
macOS 10.14.5

Feedback

    val result: Any = httpClient.post {
      url(server.baseApiUrl + "/apiPath")
      body = request
      contentType(ContentType.Application.Json)
    }

With the above code, we found that our app doesn't throw http exceptions, which was a bit unexpected, and took a bit of debugging. result ends up being a DelegatedResponse, and is thus expecting us to handle the http value ourselves, which was again, unexpected.

This code was something we didn't care about the response, and so someone who didn't know what they were doing defaulted it to type Any.

Instead of unexpectedly returning a DelegatedResponse object, and expecting the caller to handle the error handling, it'd probably be better to throw a RuntimeException mentioning this isn't an allowed response type, so that unexpected bugs like this don't happen.

@ScottPierce ScottPierce changed the title Client doesn't throw for HTTP Exceptions when Any is the expected result. Client doesn't throw for HTTP Errors when Any is the expected result. Jun 27, 2019
@e5l e5l self-assigned this Jun 28, 2019
@e5l e5l added the ux User Experience issue label Jun 28, 2019
@e5l
Copy link
Member

e5l commented Jun 28, 2019

Hi, @ScottPierce, thanks for the report. Probably we should prevent to use Any as the receive type.

@ScottPierce
Copy link
Author

@e5l I agree

@oleg-larshin
Copy link

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

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

No branches or pull requests

3 participants