restlet / restlet-framework-java

The first REST API framework for Java
https://restlet.talend.com
646 stars 284 forks source link

org.restlet.resources.ResourceException is not serializable but implements Serializable interface #1402

Open tomas-muller opened 1 year ago

tomas-muller commented 1 year ago

ResourceException cannot be serialized even so it implements the Serializable interface.

Caused by: java.io.NotSerializableException: org.restlet.Request
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1175) ~[?:?]

Please mark the non-serializable internal attributes (status, request, and message) as transient so that the error message and stack trace can be serialized (and, for example, passed over the network).

Tembrel commented 1 year ago

ResourceException implements Serializable only because it extends RuntimeException, and it provides a serialVersionUID to avoid a warning, but the standard advice for years has been to use something other than Java serialization for sending Java over the wire.

Making some fields transient might provide an acceptable default serialization for you, but it's not necessarily going to satisfy other users. I suppose a disclaimer might help, e.g.,

This class makes a half-hearted effort to allow default Java serialization to work, but for production uses, consider using library designed explicitly for serializing Java objects.

But I'd recommend leaving this alone, and not raise any false hopes.

tomas-muller commented 1 year ago

Well, we use Restlet to make API calls to external services along a cluster of machines (using JGrouips and RPC dispatcher). The response is NOT using Java serialization, but when an API call fails, the exception returned gets sent over instead (the ResourceException is passed as the cause). As ResourceException does NOT provide any other serialization means, and it implements the Serializable interface, the dispatcher attempts to send it instead and fails.

Marking the method as transient is a standard way of notifying the Java serialization that the attribute cannot be serialized. What is wrong with doing that??? The same happens with the exception's backtrace and depth already.

Tembrel commented 1 year ago

Nothing wrong with it, but I worry that this will encourage people to rely on the default serialization of ResourceException in other contexts. What if someone else's notion of what needs to be serialized is different from yours?

I'd want to push for a prominent disclaimer.

tomas-muller commented 1 year ago

I have no issue with adding a disclaimer.

I understand someone may expect the non-serializable attributes to be included. On the other hand, not including them still provides a far better outcome than causing a NotSerializableException.

Having a non-serializable not-transient attribute on a class that implements a Serializable interface (even indirectly) should be a warning, similar to when the serialVersionUID is not present.