Distinguishing between a network error and a conversion error

[dbyron-sf]

What kind of issue is this?

• Question. This issue tracker is not the place for questions. If you want to ask how to do
  something, or to understand why something isn't working the way you expect it to, use Stack
  Overflow. https://stackoverflow.com/questions/tagged/retrofit

• Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests
  get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

• Feature Request. Start by telling us what problem you’re trying to solve. Often a solution
  already exists! Don’t send pull requests to implement new features without first getting our
  support. Sometimes we leave features out on purpose to keep the project small.

I can't quite tell if #3749 already covers this, or perhaps it's a big enough change that it belongs in the retrofit 3.x issue. What I'm looking for is a way to tell explicitly when there's a "conversion exception". As in, when responseConverter.convert throws an exception. The interface specifies an IOException, but because Call.execute calls both parseResponse and okhttp's Call.execute which also throws an IOException, I can't see a way for callers of Call.execute to be able to tell the difference.

The motivation is to be able to tell whether it's worth retrying a request or not. I wouldn't expect conversion exception to be worth retrying -- they'd fail every time. But other IOExceptions would be worth retrying as they might succeed.

Defining an exception class that extends from IOException, and then changing Converter.convert to throw that feels like it would solve this, but there could totally be better ways, or things I haven't thought of. Happy to make a PR if this seems like a reasonable approach.

[dbyron-sf]

I took a crack at this with #3974

[JakeWharton]

If the conversion library is well-behaved / well-designed it will throw a non-IOException for serialization problems. For example, Moshi throws JsonDataException for serialization problems but propagates IOException for transport issues. Less well-designed libraries like Gson throw a JsonIOException which is a subclass of IOException making identification of these problems more challenging since you need to actually know about the special subtype rather than simply having a condition on whether or not the type is an IOException subclass or not.

In the linked PR you actually are converting all transport issues into conversion exceptions because we stream the response body to the converter. This means as deserialization of the response body is being performed, the library is also reading the bytes directly from the underlying socket where network problems will cause an IOException. Wrapping it unconditionally in a conversion exception would be extremely misleading since the problem almost always originates with the network having the opposite of your desired effect.

I don't think there's any action to take here since the Retrofit was already designed to distinguish between these cases using IOException vs. non-IOException, but we do still rely on the design of the converter library to also be designed to distinguish between these cases. In practice we've found that most handle this correctly, with Gson being a notable exception.

[dbyron-sf]

Fair enough. Maybe jackson falls into the less-well-designed category? It does seem to work fairly hard to preserve IOExceptions, at least in DefaultSerializerProvider it does, but then JacksonException inherits from IOException. At least for retrofit, (I think) it's sufficient to catch JsonProcessingException or maybe JacksonException.

[JakeWharton]

I don't have any experience with Jackson so yeah not familiar with how it behaves.

Another way to think about the behavior with Retrofit is that it behaves the same way as if you were calling into the serialization library directly. We try not to obscure the underlying behavior which can sometimes be useful and sometimes be challenging, but ultimately it really comes down to however that library was designed to report problems.