google / gson

A Java serialization/deserialization library to convert Java Objects into JSON and back
Apache License 2.0
23.4k stars 4.29k forks source link

Support of serdes for Throwable/Exception no longer works since Java 17 #2352

Open q3769 opened 1 year ago

q3769 commented 1 year ago

Gson version

2.10.1

Java / Android version

17.0.6 Amazon Corretto

Used tools

Description

    public static void main(String[] args) {
        System.out.println("11111111111111111111111111111111111111111 " + new Gson().toJson(new ExBean()));
    }

    static class ExBean {
        Throwable throwable = new Exception("xxxxxxxxxxxxxxxxx");
    }

Expected behavior

Gson should serialize the ExBean instance with the Throwable field, with stack trace details and related info on the Exception.

Actual behavior

Gson.toJson errors out with exception. See stack trace below.

Reproduction steps

  1. paste this in a class
    public static void main(String[] args) {
        System.out.println("11111111111111111111111111111111111111111 " + new Gson().toJson(new ExBean()));
    }

    static class ExBean {
        Throwable throwable = new Exception("xxxxxxxxxxxxxxxxx");
    }
  1. Run the main method

Exception stack trace

Exception in thread "main" com.google.gson.JsonIOException: Failed making field 'java.lang.Throwable#detailMessage' accessible; either increase its visibility or write a custom TypeAdapter for its declaring type.
    at com.google.gson.internal.reflect.ReflectionHelper.makeAccessible(ReflectionHelper.java:38)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:286)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:130)
    at com.google.gson.Gson.getAdapter(Gson.java:556)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.createBoundField(ReflectiveTypeAdapterFactory.java:160)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:294)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:130)
    at com.google.gson.Gson.getAdapter(Gson.java:556)
    at com.google.gson.Gson.toJson(Gson.java:834)
    at com.google.gson.Gson.toJson(Gson.java:812)
    at com.google.gson.Gson.toJson(Gson.java:759)
    at com.google.gson.Gson.toJson(Gson.java:736)
    at elf4j.engine.writer.pattern.JsonPatternSegment.main(JsonPatternSegment.java:129)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private java.lang.String java.lang.Throwable.detailMessage accessible: module java.base does not "opens java.lang" to unnamed module @136432db
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
    at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
    at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
    at com.google.gson.internal.reflect.ReflectionHelper.makeAccessible(ReflectionHelper.java:35)
    ... 12 more
Marcono1234 commented 1 year ago

Have added the "enhancement" label because there was never really proper support for Throwable; it just accessed the internal fields of the JDK classes. And that was already flawed; if Throwable.getStackTrace() had not been called (indirectly) before the exception was passed to Gson, the JSON output would contain "stackTrace":[] because the stack trace had not been initialized yet. It also did not include the exception type, so if the exception message itself was not meaningful enough (e.g. NullPointerException might not have any message at all), you would have no clue what kind of exception occurred.

Backward compatibility might also be a (small) problem: If Gson added now a type adapter factory which handles Throwable and all subclasses, it would not include custom fields from subclasses anymore.

Most likely it is also not possible to properly support deserialization because exceptions can have arbitrary constructors (which might make adjustments to the provided message), so even if Gson was able to create an instance of an exception, the instance would most likely not be identical to the original one.

Note that I am not a direct member of this project, but my opinion is that users should write their own adapter, and include all information they consider important. Here is an example for including the type name, the message, the cause and suppressed exceptions:

class ThrowableAdapterFactory implements TypeAdapterFactory {
  private ThrowableAdapterFactory() {}

  public static final ThrowableAdapterFactory INSTANCE = new ThrowableAdapterFactory();

  @Override
  public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
    // Only handles Throwable and subclasses; let other factories handle any other type
    if (!Throwable.class.isAssignableFrom(type.getRawType())) {
      return null;
    }

    @SuppressWarnings("unchecked")
    TypeAdapter<T> adapter = (TypeAdapter<T>) new TypeAdapter<Throwable>() {
      @Override
      public Throwable read(JsonReader in) throws IOException {
        throw new UnsupportedOperationException();
      }

      @Override
      public void write(JsonWriter out, Throwable value) throws IOException {
        if (value == null) {
          out.nullValue();
          return;
        }

        out.beginObject();
        // Include exception type name to give more context; for example NullPointerException might
        // not have a message
        out.name("type");
        out.value(value.getClass().getSimpleName());

        out.name("message");
        out.value(value.getMessage());

        Throwable cause = value.getCause();
        if (cause != null) {
          out.name("cause");
          write(out, cause);
        }

        Throwable[] suppressedArray = value.getSuppressed();
        if (suppressedArray.length > 0) {
          out.name("suppressed");
          out.beginArray();

          for (Throwable suppressed : suppressedArray) {
            write(out, suppressed);
          }

          out.endArray();
        }
        out.endObject();
      }
    };
    return adapter;
  }
}

And then register it like this:

new GsonBuilder()
  .registerTypeAdapterFactory(ThrowableAdapterFactory.INSTANCE)
  .create()

Edit: You could probably also simplify this to only write a TypeAdapter with the above code instead of a TypeAdapterFactory and then register it with GsonBuilder.registerTypeHierarchyAdapter instead.

q3769 commented 1 year ago

Thank you @Marcono1234 I appreciate the explanation and code sample.

I see this would be messy now that you mentioned the deserialization side of the issue.

BTW, is there an easier (more relaxed) way to register user customized adapters? For example, the Adapter interface has both read and write methods but I see little reason to force the user to implement both if they only need either serialization or deserialization. Wouldn't being able to register serializer/writer and deserializer/reader separately more convenient?

Marcono1234 commented 1 year ago

BTW, is there an easier (more relaxed) way to register user customized adapters? For example, the Adapter interface has both read and write methods but I see little reason to force the user to implement both if they only need either serialization or deserialization. Wouldn't being able to register serializer/writer and deserializer/reader separately more convenient?

There are the separate interfaces JsonSerializer and JsonDeserializer, and for this use case here you could register them with GsonBuilder.registerTypeHierarchyAdapter[^1], or for a specific type with GsonBuilder.registerTypeAdapter​. However, as mentioned in the documentation of these interfaces they are less efficient than TypeAdapter because they work on a complete in-memory representation of the JSON data in the form of a JsonElement object. Additionally, compared to TypeAdapterFactory they don't support delegating to the built-in adapter for the same type, e.g. to perform pre-processing.

The easiest solution would probably be to implement TypeAdapter but let its write respectively read method unconditionally throw an exception.

[^1]: Actually, maybe even the code in my comment above could directly register the TypeAdapter with registerTypeHierarchyAdapter and would not need a TypeAdapterFactory.