SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached

[epugh]

https://issues.apache.org/jira/browse/SOLR-18324

Summary

(Supersedes #4674, which was accidentally auto-closed when its branch got renamed outside of GitHub's branch-rename API rather than through it — same content, just corrected to reference the actual JIRA ticket number, SOLR-18324, instead of the SOLR-18322 placeholder used in the original commit message.)

See the reproduction script that is attached to the JIRA issue that can be used to demonstrate the bug on main and it being fixed on this branch.

Discovered while investigating an intermittent CollectionsApi.GetCollectionStatus failure under basic-auth-secured clusters (used while migrating bin/solr delete off ZooKeeper). The endpoint would fail intermittently with:

SEVERE: An exception has been thrown from an exception mapper class org.apache.solr.jersey.CatchAllExceptionMapper.
java.lang.NullPointerException: Cannot invoke "org.apache.solr.request.SolrQueryRequest.getCore()" because "req" is null
	at org.apache.solr.handler.RequestHandlerBase.processReceivedException(RequestHandlerBase.java:326)
	at org.apache.solr.jersey.CatchAllExceptionMapper.processAndRespondToException(CatchAllExceptionMapper.java:93)

...cascading into Jersey re-invoking the response filter chain via its "already mapped exception" fallback path, which in turn caused RequestMetricHandling.PostRequestMetricsFilter to call .stop() a second time on an already-stopped timer — tripping RTimer's assert state == STARTED || state == PAUSED (only visible with assertions enabled, e.g. under -ea test runs; silently harmless in production builds where the assert is compiled out).

Root cause: when a request fails before V2HttpCall attaches a SolrQueryRequest to the Jersey request context (the actual trigger appears to be a rare race in inter-node request handling — not fully root-caused, but independent of any specific endpoint), several Jersey filters/mappers in org.apache.solr.jersey unconditionally dereference that possibly-null SolrQueryRequest, causing a second, cascading exception while trying to handle the first one.

Changes

• RequestHandlerBase.processReceivedException: null-check req before calling req.getCore().
• CatchAllExceptionMapper.buildExceptionResponse: null-check solrQueryRequest before calling .getParams().
• PostRequestDecorationFilter / PostRequestLoggingFilter: null-check solrQueryRequest before dereferencing (mirrors the existing, correct null-check already present in MediaTypeOverridingFilter, which had a // TODO Is it valid for SQRequest to be null? comment confirming this was already a known possibility).
• RequestMetricHandling.PostRequestMetricsFilter: guards against being invoked more than once for the same request by clearing the TIMER property after stopping it, so a second (re-entrant) invocation is a no-op instead of crashing.

Test plan

• New unit tests: RequestHandlerBaseTest (null req cases), PostRequestLoggingFilterTest, PostRequestDecorationFilterTest (new), RequestMetricHandlingTest (new) — each verified to fail without the corresponding fix and pass with it
• Existing CatchAllExceptionMapperTest (SOLR-18066) still passes — no regression to hideStackTrace handling
• Reproduced the original intermittent failure with a throwaway integration test hitting GetCollectionStatus under a SecurityJson.SIMPLE-secured 2-node cluster (~25-50% failure rate over multiple runs); with these fixes applied, 20+ consecutive runs passed
• ./gradlew :solr:core:spotlessJavaCheck clean
• ./gradlew :solr:core:compileJava :solr:core:compileTestJava clean

🤖 Generated with Claude Code

[dsmiley]

didn't look at the tests much

[dsmiley]

trace

[dsmiley]

TBH... I have little faith in unit testing low level plumbing like this. IMO they even have negative value as it's yet another thing to change if we change the plumbing. It's better to accomplish the high level goals (like what led you to uncover this bug). My comment applies to all your tests here. Just because you write a line of code doesn't mean it needs a direct test.

[epugh]

Thanks @dsmiley for taking a look. Yeah, I don't know that I feel comfortable with saying "yes, I found a bug" and or even "yes, this is hte fix we want".