You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "epugh (via GitHub)" <gi...@apache.org> on 2026/07/27 16:52:27 UTC

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

epugh opened a new pull request, #4676:
URL: https://github.com/apache/solr/pull/4676

   ## 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.)
   
   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
   - [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 (~25-50% failure rate over multiple runs); with these fixes applied, 20+ 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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4676:
URL: https://github.com/apache/solr/pull/4676#issuecomment-5110264811

   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".   


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4676:
URL: https://github.com/apache/solr/pull/4676#issuecomment-5892811605

   > good to know the bug is fixed but lets not incur the burden of this test henceforth
   
   i'm going to pare this back.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4676:
URL: https://github.com/apache/solr/pull/4676#discussion_r4139856754


##########
changelog/unreleased/SOLR-18324-jersey-null-request-npe.yml:
##########
@@ -0,0 +1,7 @@
+title: Fixed a race condition where V2 (JAX-RS) API requests that fail before a SolrQueryRequest is attached to the request context could throw a NullPointerException while building the error response, which could cascade into an internal request-metrics-timer assertion failure under Solr's security manager / assertions-enabled test builds.

Review Comment:
   minor nit: this is a long explanation for an NPE in a changelog -- great explanation for a PR description, say.  I suggest simply:
   
   > The V2 JAX-RS API, when faced with an error, could throw an NPE masking the true problem.



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #4676:
URL: https://github.com/apache/solr/pull/4676


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "sigram (via GitHub)" <gi...@apache.org>.
sigram commented on PR #4676:
URL: https://github.com/apache/solr/pull/4676#issuecomment-5662771247

   The null checks in these error paths are not expensive, I think we should add them. I kind of agree that too many low level unit tests may become a nuisance .. but the other tests here verify end-user expectations wrt. the returned error info so I think they are useful.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4676:
URL: https://github.com/apache/solr/pull/4676#discussion_r3661950963


##########
solr/core/src/test/org/apache/solr/jersey/PostRequestDecorationFilterTest.java:
##########


Review Comment:
   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.



##########
solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java:
##########
@@ -55,6 +55,10 @@ public void filter(
     }
     final SolrQueryRequest solrQueryRequest =
         (SolrQueryRequest) requestContext.getProperty(SOLR_QUERY_REQUEST);
+    if (solrQueryRequest == null) {
+      log.debug("Skipping QTime assignment because no SolrQueryRequest was attached");

Review Comment:
   trace



-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4676:
URL: https://github.com/apache/solr/pull/4676#issuecomment-5901756882

   @dsmiley i have addressed your feedback and I think this ready for merging.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #4676:
URL: https://github.com/apache/solr/pull/4676#issuecomment-5901565990

   Okay, pare-ing knife applied.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org