SOLR-3284: fix RemoteSolrException NPE when remoteError is null#4637
Conversation
The constructor built its details map with Map.of, which rejects null values, so it threw NullPointerException when remoteError was null. That happens whenever a non-OK response has no parseable error body (e.g. a proxy error), where it masked the actual HTTP status. Use a null-tolerant map instead.
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
+1 thanks.
Prefer no changelog...
David preferred no changelog for this small NPE fix.
|
Removed the changelog entry. Also checked the Crave CI red — it's IndexFetcherPacketProtocolTest ( |
|
correction on the CI red — it's not a flake. IndexFetcherPacketProtocolTest fails deterministically on clean main (no changes) with the CI seed the |
|
filed the fix for that pre-existing test failure: #4642 (SOLR-18098). forces an FS directory factory in IndexFetcherPacketProtocolTest so the randomized RAM directory doesn't hide the staged file. this PR's CI red was entirely that test. |
2 participants
Follow-up to the SOLR-3284 error path (David flagged this on #4632 as worth its own change).
RemoteSolrException(remoteHost, code, remoteError, skipRetry)built its details withMap.of(...), which rejects null values, so it threwNullPointerExceptionwhenremoteErrorwas null.A null
remoteErroris expected:ConcurrentUpdateBaseSolrClientcalls this constructor in afinallyblock withremoteErrorstill null whenever a non-OK response has no parseable error body (a proxy 502/503, a truncated or non-Solr response). The NPE then masked the actual HTTP status the exception was meant to carry.HttpSolrClienthas a similar call site.Switched to
Utils.makeMap(the map building idiom already used elsewhere in the code), which tolerates a null value. AddedRemoteSolrExceptionTestcovering the null case (the regression) plus the consumers of the resulting exception, and the non-null case.