You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "joaohmalves (via GitHub)" <gi...@apache.org> on 2026/05/29 01:52:31 UTC
[PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
joaohmalves opened a new pull request, #16298:
URL: https://github.com/apache/dubbo/pull/16298
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 `IOException` containing the class name and
`must implement java.io.Serializable` is thrown at encode time in both
request and response paths.
### What is changed?
- `DubboCodec.encodeRequestData()`: wraps `writeObject()` in a
try/catch that intercepts the internal `IllegalArgumentException` and
rethrows a clear `IOException` with the offending class name.
- `DubboCodec.encodeResponseData()`: same treatment for the response path.
- `DubboProtocolTest`: re-enabled `testNonSerializedParameter` and
`testReturnNonSerialized` which were previously `@Disabled` because
the expected behavior was not implemented.
### Verifying this change
All tests in `dubbo-rpc-dubbo` module pass locally, including the
two previously disabled tests that now validate the correct behavior.
## Checklist
- [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project.
- [x] Make sure gitHub actions can pass. [Why the workflow is failing and how to fix it?](../CONTRIBUTING.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw closed pull request #16298: fix: throw clear exception when request parameter is not Serializable
URL: https://github.com/apache/dubbo/pull/16298
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #16298:
URL: https://github.com/apache/dubbo/pull/16298#issuecomment-4571460868
Have you checked the situation of the issue?I couldn't reproduce. Moreover, it seems that such changes risk swallowing non-Serializer exceptions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
Posted by "joaohmalves (via GitHub)" <gi...@apache.org>.
joaohmalves commented on PR #16298:
URL: https://github.com/apache/dubbo/pull/16298#issuecomment-4573958742
After checking a bit further, I was unable to reproduce the exact NPE from the stack trace (GroupServiceKeyCache.getServiceKey).
What I found instead is that Dubbo already detects the non-Serializable class in DefaultSerializeClassChecker and logs an ERROR, but does not propagate a clear exception.
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #16298:
URL: https://github.com/apache/dubbo/pull/16298#issuecomment-4588923729
As for me, https://github.com/apache/dubbo/pull/16299 might be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: throw clear exception when request parameter is not Serializable [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16298:
URL: https://github.com/apache/dubbo/pull/16298#issuecomment-4569992639
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/16298?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `63.15789%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 60.75%. Comparing base ([`abf4e98`](https://app.codecov.io/gh/apache/dubbo/commit/abf4e985815aa70aaa79294b680f0ad814a2f45d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6605aa2`](https://app.codecov.io/gh/apache/dubbo/commit/6605aa2fe66a06c8f718b730b52fd6b37feb5386?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/16298?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/apache/dubbo/rpc/protocol/dubbo/DubboCodec.java](https://app.codecov.io/gh/apache/dubbo/pull/16298?src=pr&el=tree&filepath=dubbo-rpc%2Fdubbo-rpc-dubbo%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Frpc%2Fprotocol%2Fdubbo%2FDubboCodec.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL0R1YmJvQ29kZWMuamF2YQ==) | 63.15% | [1 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/16298?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.18% <52.63%> (+0.05%)` | :arrow_up: |
| [integration-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [samples-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.21% <26.31%> (+0.03%)` | :arrow_up: |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.83% <26.31%> (+0.07%)` | :arrow_up: |
| [unit-tests-java11](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.01% <47.36%> (-0.04%)` | :arrow_down: |
| [unit-tests-java17](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.49% <47.36%> (+0.01%)` | :arrow_up: |
| [unit-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.49% <47.36%> (-0.03%)` | :arrow_down: |
| [unit-tests-java25](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.46% <47.36%> (-0.01%)` | :arrow_down: |
| [unit-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16298/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.05% <47.36%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo/pull/16298?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org