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

[Runtime] throw ApolloNetworkException instead of ApolloParseException on IOException #5068

Closed
wants to merge 3 commits into from

Conversation

martinbonnin
Copy link
Contributor

Low hanging fruit, closes #2783.

@martinbonnin martinbonnin requested a review from BoD as a code owner July 6, 2023 13:13
@netlify
Copy link

netlify bot commented Jul 6, 2023

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 874906c
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/64a6d99e2031d60007b7b1ea

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

ApolloParseException(
message = "EOFException while parsing the HTTP network response",
cause = throwable
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting one. It could be argued that EOFException is a network exception if the stream is closed too early

  • If there is a Content-Length this doesn't happen because OkHttp has a specific java.net.ProtocolException for this
  • I'm not 100% sure about chunked encoding
  • Also not 100% clear about the other engines (Ktor, etc...)

@martinbonnin
Copy link
Contributor Author

Turns out it's not low hanging fruits and this raises more questions. Since it's not critical, I'll close this until we refine the exception handling of HttpEngine

@martinbonnin martinbonnin deleted the better-exception branch July 24, 2023 16:31
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

Successfully merging this pull request may close these issues.

ApolloParseException caused by SocketTimeoutError while reading response body
2 participants