google / google/gson

Create proper unchecked exception hierarchy

Open
#2,359 0 comments 1 reaction 0 assignees View on GitHub
enhancement
Dominant language
Java
Stars
24.2k
Forks
4.5k
Avg merge
6d 4h
Merged PRs (30d)
12

Description

# Problem solved by the feature
Currently Gson's unchecked exception hierarchy looks like this:
- `JsonParseException`
- `JsonIOException`
- `JsonSyntaxException`

This has the following issues:
- `JsonIOException` is a subclass of `JsonParseException`, however `JsonIOException` should also be usable for serialization
- Multiple use cases are not covered be these, leading to misuse in Gson's code base, usage of `RuntimeException` or other `java.lang` exceptions (slightly related: #548)

This mainly or only affects `Gson` and built-in adapters. Other API such as `JsonReader`, `JsonWriter` and `JsonElement` (and subclasses) are quite consistent with the exceptions they throw. For checked exceptions Gson only has `MalformedJsonException` which is only used by `JsonReader` so there is probably no need for changes there (and changes would most likely also be source incompatible due to it being a checked exception).

# Feature description
Define a proper hierarchy for unchecked exceptions. Maybe something like the following would be useful, but I have not thought that much about it yet:

- `GsonException`
- `JsonIOException`
- `DeserializationException` (or keep using existing `JsonParseException` for this)
- `JsonSyntaxException`
- `SerializationException` (or `JsonSerializionException` or similar to match `JsonParseException`)
- `TypeAdapterException`, for all issues which occur during creation of type adapters

Then adjust the Gson code to throw proper and consistent exceptions, mainly / only `Gson` and built-in adapters. Existing exception classes could be kept but their superclasses might have to be adjusted in some cases.

`GsonException` and all subclasses can be used by user code as well. Since it is likely that user-written adapters throw arbitrary runtime exceptions though, and that for malformed JSON built-in and user-written adapters might throw other runtime exceptions as well, the documentation for `GsonException` should probably recommend that users should not only catch `GsonException` but any `RuntimeException` to be safe.

Feedback is highly appreciated!

## Risks
- This will be behavior-wise a breaking change (though hopefully makes usage easier and more consistent in the future)
- If the superclass of `JsonIOException` is changed that could become a source (or even binary) incompatible change; though maybe only in rare cases where the exception is stored in some way (e.g. a local variable with the superclass as type)
- If `JsonIOException` is not a subclass of `JsonParseException` anymore, then users cannot catch a single exception to cover both IO and non-IO exceptions anymore, or would have to catch `GsonException` (though maybe that is not such a big problem)

# Alternatives / workarounds
- Only extend the existing exception hierarchy with additional classes, but don't change any existing ones.
This would keep the problem that `JsonIOException` can also occur during serialization and therefore `JsonParseException` is not an ideal superclass for it.
- Do nothing, and maybe just recommend that users catch `RuntimeException` when calling Gson methods. That would then also cover exceptions thrown (implicitly, e.g. `NullPointerException`) by built-in or custom user-defined adapters which might currently not be wrapped by Gson in some cases.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.