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

Make FeignException truly serializable #1193

Closed
Nikolas-Charalambidis opened this issue Mar 18, 2020 · 5 comments
Closed

Make FeignException truly serializable #1193

Nikolas-Charalambidis opened this issue Mar 18, 2020 · 5 comments
Labels
enhancement For recommending new capabilities proposal Proposed Specification or API change

Comments

@Nikolas-Charalambidis
Copy link
Contributor

Nikolas-Charalambidis commented Mar 18, 2020

The class FeignException implements Serializable inherited from the Exception hierarchy, however, it contains the object feign.Request which is not Serializable itself, therefore further serialization into either JSON or Camunda process variable is impossible (my case), etc...

Is possible to either change the internals of feign.Request to be serializable itself or amend FeignException to hold information about the failed request in another way?

Version: 10.4.0

@kdavisk6 kdavisk6 added enhancement For recommending new capabilities proposal Proposed Specification or API change labels Mar 29, 2020
@kdavisk6
Copy link
Member

@Nikolas-Charalambidis

I think we would ok with either option. The simplest would be to investigate what it would take to make Request Serializable. Would you be willing to look into that for us?

@Nikolas-Charalambidis
Copy link
Contributor Author

Nikolas-Charalambidis commented Mar 30, 2020

I have already looked into the internals of feign.Request (and its feign.template.Template) which use objects right from JDK such as java.nio.charset.Charset that is not Serializable itself...

I think there are two ways:

  1. Make Request as transient in the FeignException.
  2. Make Request implement Serializable and all instance field classes Serializable/transient.

The object can never be fully Serializable when it uses java.nio.charset.Charset as an instance field internally.

Edit: I can make a pull-request, but I'd like to discuss the best way to go first.

@kdavisk6
Copy link
Member

kdavisk6 commented Apr 6, 2020

@Nikolas-Charalambidis

I think it's reasonable enough for us to change the charset instance variable type, so long as the method interfaces don't change. Are there any other instance variables that pose a problem?

@Nikolas-Charalambidis
Copy link
Contributor Author

@kdavisk6

I have noticed no more problematic classes aside from java.nio.charset.Charset. We can substitute it with either a String literal or an enumeration from which we would be able to construct Charset where needed and keeping the interfaces untouched. If I propose a pull request, would you check it up?

@kdavisk6
Copy link
Member

Closed via #1262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For recommending new capabilities proposal Proposed Specification or API change
Projects
None yet
Development

No branches or pull requests

2 participants