fix: throw clear exception when request parameter is not Serializable#16298
fix: throw clear exception when request parameter is not Serializable#16298joaohmalves wants to merge 2 commits into
Conversation
Previously, passing a non-Serializable parameter to a Dubbo service resulted in a confusing error message. Now a clear IOException with the class name and 'must implement java.io.Serializable' is thrown at encode time in both request and response paths. Fixes apache#16293
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16298 +/- ##
============================================
- Coverage 60.78% 60.75% -0.03%
+ Complexity 11766 11751 -15
============================================
Files 1953 1953
Lines 89186 89203 +17
Branches 13454 13462 +8
============================================
- Hits 54208 54196 -12
- Misses 29411 29433 +22
- Partials 5567 5574 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Have you checked the situation of the issue?I couldn't reproduce. Moreover, it seems that such changes risk swallowing non-Serializer exceptions? |
|
After checking a bit further, I was unable to reproduce the exact NPE from the stack trace (GroupServiceKeyCache.getServiceKey). Also as you pointed out this fix does risks swallowing unrelated exceptions so I would like to work on it a bit more. As this is my first contribution to this project, would it make sense for me to go back to the issue and ask the reporter for more details on how to reproduce the exact stack trace before reworking this PR? |
Previously, passing a non-Serializable parameter to a Dubbo service resulted in a confusing error message. Now a clear IOException with the class name and 'must implement java.io.Serializable' is thrown at encode time in both request and response paths.
Fixes #16293
What is the purpose of the change?
This PR fixes a misleading error message when calling a Dubbo service
with a parameter or return value that does not implement
java.io.Serializable.Previously, the error was buried inside internal serialization layers
and did not clearly indicate the root cause, making it hard to diagnose.
Now a clear
IOExceptioncontaining the class name andmust implement java.io.Serializableis thrown at encode time in bothrequest and response paths.
What is changed?
DubboCodec.encodeRequestData(): wrapswriteObject()in atry/catch that intercepts the internal
IllegalArgumentExceptionandrethrows a clear
IOExceptionwith the offending class name.DubboCodec.encodeResponseData(): same treatment for the response path.DubboProtocolTest: re-enabledtestNonSerializedParameterandtestReturnNonSerializedwhich were previously@Disabledbecausethe expected behavior was not implemented.
Verifying this change
All tests in
dubbo-rpc-dubbomodule pass locally, including thetwo previously disabled tests that now validate the correct behavior.
Checklist