JsonReader#beginObject should throw checked exceptions, instead throws runtime IllegalStateException.

[grantamos]

Gson version

2.9.0

Java / Android version

Unsure - if this is needed please let me know & I can dig this up, though it should be of no consequence.

Used tools

Nothing of consequence to this bug.

Description

When parsing an JSON object that fails due to the value being an incorrect type, an unchecked exception is thrown.

beginObject specifies a checked exception of type IOException here:

gson/gson/src/main/java/com/google/gson/stream/JsonReader.java

Line 454 in 18b0892

public void beginObject() throws IOException {

When it fails to find an object, however, it throws an IllegalStateException here:

gson/gson/src/main/java/com/google/gson/stream/JsonReader.java

Line 463 in 18b0892

throw unexpectedTokenError("BEGIN_OBJECT");

and here:

gson/gson/src/main/java/com/google/gson/stream/JsonReader.java

Line 1744 in 18b0892

private IllegalStateException unexpectedTokenError(String expected) throws IOException {

This isn't caught at compile time because IllegalStateException is a RuntimeException. Note that unexpectedTokenError says it throws IOException but the IDE warns this throws is unused.

Expected behavior

We expected an IOException to be thrown. Else, the beginObject method should check any other exceptions.

Actual behavior

An unchecked runtime exception was thrown.

Reproduction steps

Attempt to parse a JSON payload expecting an object but finding another type.

e.g.

JSON PAYLOAD:
{
  "this_key_is_expected_to_be_an_object": false
}

Java class:
class Payload {
   public NestedObject this_key_is_expected_to_be_an_object;
}

class NestedObject {
  public boolean inner_field;
}

Exception stack trace

java.lang.IllegalStateException: Expected BEGIN_OBJECT but was BOOLEAN at line 1 column 2691 path _REDACTED_
        at com.google.gson.stream.JsonReader.beginObject(JsonReader.java:393)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:59)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:25)
        at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:204)
        at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:204)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:68)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:25)
        at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:204)
        at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:204)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:68)
        at com.squareup.wire.MessageTypeAdapter.read(MessageTypeAdapter.kt:25)
        at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:204)
        at retrofit2.converter.gson.GsonResponseBodyConverter.convert(GsonResponseBodyConverter.java:39)
        at retrofit2.converter.gson.GsonResponseBodyConverter.convert(GsonResponseBodyConverter.java:27)
        at retrofit2.OkHttpCall.parseResponse(OkHttpCall.java:227)
        at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:142)
        at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
        at java.lang.Thread.run(Thread.java:764)

[Marcono1234]

beginObject specifies a checked exception of type IOException here
[...]
Note that unexpectedTokenError says it throws IOException but the IDE warns this throws is unused.

They have throws IOException because reading (which occurs when doPeek() / peek() is called) can throw an IOException.

To me it seems it is intentional that an IllegalStateException is thrown. Whether it would have been more consistent to throw an IOException or a custom subclass since a JSON syntax error causes an IOException (more specifically MalformedJsonException) is a different question. But this can probably not be changed easily anymore either since it would be backward incompatible for users who expect an IllegalStateException respectively don't expect an IOException in that case.

For some of the JsonReader methods the documentation explicitly mentions the IllegalStateException: nextString(), nextBoolean(), nextNull(), nextDouble(), nextLong(), nextInt()

So to me the issue here seems to rather be that the other JsonReader methods (such as beginObject you mentioned) don't have the IllegalStateException documented.

Would you like to create a pull request for this (see also the contributing guide)?
That would probably involve:

• Adding @throws IllegalStateException to all JsonReader methods which throw this exception on token mismatch
• Maybe: Remove the "or if this reader is closed" from the JsonReader method documentation and instead add something generic as Using the reader after {@link #close()} has been called will cause an {@link IllegalStateException}. to the class documentation right above the "Parsing JSON" header (or alternatively / additionally to the close() documentation).
  Because this affects more than just these value reading methods, and documenting it separately for all affected methods might be a bit verbose.

[Marcono1234]

Have created #2670 now to improve the JsonReader documentation.