Fix misleading NPE when request parameter is not Serializable#16299
Fix misleading NPE when request parameter is not Serializable#16299chensishang wants to merge 2 commits into
Conversation
When a Dubbo service is called with non-serializable parameters, the invocation deserialization fails silently and `path` becomes null. This null path is then passed to GroupServiceKeyCache which uses a ConcurrentHashMap that does not accept null keys, resulting in a confusing NullPointerException. Added a null check for `path` in DubboProtocol.getInvoker() to throw a clear RemotingException with a meaningful message pointing users to the actual cause (non-serializable parameters). Fixes apache#16293 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16299 +/- ##
============================================
- Coverage 60.80% 60.77% -0.04%
- Complexity 11766 11771 +5
============================================
Files 1953 1953
Lines 89188 89191 +3
Branches 13454 13455 +1
============================================
- Hits 54235 54206 -29
- Misses 29391 29414 +23
- Partials 5562 5571 +9
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:
|
|
Hi reviewers, I noticed the Codecov check failed with "0% coverage". After checking, this seems to be due to an incomplete report upload ("HEAD比BASE少7次上传") rather than a problem with the code change itself. I have already added a unit test (testGetInvokerThrowsOnNullPath) which covers the new null-check logic, and it passes locally. The Codecov result appears to be a false negative caused by the CI infrastructure. Could you please take another look? Thanks! |
|
Thanks for the review! |
|
Hi @zrlw, thanks for the review! The PR contains two commits: the actual fix ( The maintainer can squash on merge if needed. Sorry for the noise! |
When a Dubbo service is called with non-serializable parameters, the invocation deserialization fails silently and
pathbecomes null. This null path is then passed to GroupServiceKeyCache which uses a ConcurrentHashMap that does not accept null keys, resulting in a confusing NullPointerException.Added a null check for
pathin DubboProtocol.getInvoker() to throw a clear RemotingException with a meaningful message pointing users to the actual cause (non-serializable parameters).Fixes #16293
What is the purpose of the change?
Checklist