epugh opened a new pull request, #4674:
URL: https://github.com/apache/solr/pull/4674
## Summary
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
- [x] New unit tests: `RequestHandlerBaseTest` (null `req` cases),
`PostRequestLoggingFilterTest`, `PostRequestDecorationFilterTest` (new),
`RequestMetricHandlingTest` (new) — each verified to fail without the
corresponding fix and pass with it (confirmed
`RequestMetricHandlingTest.testPostRequestMetricsFilterToleratesBeingInvokedTwice`
fails on `git stash` of just the `RequestMetricHandling.java` fix, passes once
restored)
- [x] Existing `CatchAllExceptionMapperTest` (SOLR-18066) still passes — no
regression to `hideStackTrace` handling
- [x] Reproduced the original intermittent failure with a throwaway
integration test hitting `GetCollectionStatus` under a
`SecurityJson.SIMPLE`-secured 2-node cluster (~40% failure rate over multiple
runs); with these fixes applied, 15/15 consecutive runs passed
- [x] `./gradlew :solr:core:spotlessJavaCheck` clean
- [x] `./gradlew :solr:core:compileJava :solr:core:compileTestJava` clean
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
