Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tomas-muller opened this issue May 9, 2023 · 4 comments

Comments

@tomas-muller
Copy link

tomas-muller commented May 9, 2023

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).

tomas-muller added a commit to tomas-muller/restlet-framework-java that referenced this issue May 9, 2023
…nsient

- this is to allow ResourceException to be serialized
- fixes issue restlet#1402
tomas-muller added a commit to tomas-muller/restlet-framework-java that referenced this issue May 9, 2023
- non-serializable internal attributes (status, request, and message) marked as transient
- this is to allow ResourceException to be serialized, passing over the message and stack-trace (which is typically all that is needed to show the error or print it in the log)
- fixes issue restlet#1402
@Tembrel
Copy link
Collaborator

Tembrel commented May 9, 2023

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
Copy link
Author

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
Copy link
Collaborator

Tembrel commented May 9, 2023

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
Copy link
Author

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.

thboileau pushed a commit that referenced this issue Jan 26, 2024
* ResourceException: avoid NotSerializableException

- non-serializable internal attributes (status, request, and message) marked as transient
- this is to allow ResourceException to be serialized, passing over the message and stack-trace (which is typically all that is needed to show the error or print it in the log)
- fixes issue #1402

* Add disclaimer about default serialization

Disclaimer needed now that default serialization silently works.

---------

Co-authored-by: Tim Peierls <237258+Tembrel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants