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

Add error body to HttpException for easier debugging #3274

Closed

Conversation

@adrw
Copy link
Member

adrw commented Jan 2, 2020

HttpExceptions are difficult to pinpoint to a specific call or what the underlying error was even though it may be present in the response.

Adding the errorBody to HttpException reveals more information that can be relevant and vital for debugging.

For example, this response includes more details in the body of why it is a bad request but that doesn't show up in the HttpException.

retrofit2.adapter.rxjava2.HttpException: HTTP 400 
       at retrofit2.adapter.rxjava2.BodyObservable$BodyObserver.onNext(BodyObservable.java:54)
       at retrofit2.adapter.rxjava2.BodyObservable$BodyObserver.onNext(BodyObservable.java:37)
       at retrofit2.adapter.rxjava2.CallEnqueueObservable$CallCallback.onResponse(CallEnqueueObservable.java:60)
       at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:129)
       at okhttp3.RealCall$AsyncCall.execute(RealCall.java:174)
       at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
       at java.lang.Thread.run(Thread.java:764)
@oldergod

This comment has been minimized.

Copy link
Member

oldergod commented Jan 2, 2020

Where are the tests Andrew ?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

JakeWharton commented Jan 2, 2020

There's a few problems with this:

  • The error body might be tens, hundreds, or millions of bytes
  • The error body may contain sensitive information (PII) that is unsafe to place in an exception message
  • The error body might not be text formatted but binary formatted
  • The error body can only be consumed once in this manner and you are exhausting it

While the last one can be worked around, the first three make this unappealing and ultimately untenable as a feature.

@JakeWharton JakeWharton closed this Jan 2, 2020
@adrw

This comment has been minimized.

Copy link
Member Author

adrw commented Jan 3, 2020

@JakeWharton @oldergod Thanks for the feedback and raising those good points! I'll find another way to build my desired functionality of HttpException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.