You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ab...@apache.org on 2026/10/06 10:45:26 UTC
(solr) 10/31: SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached (#4676)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch branch_10_1
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 83a65e6c4b2bfd33731531955d73bde1fb6a7a39
Author: Eric Pugh <ep...@opensourceconnections.com>
AuthorDate: Wed Sep 30 12:54:43 2026 -0400
SOLR-18324: Fix NPE/RTimer assertion when a V2 request fails before a SolrQueryRequest is attached (#4676)
Co-authored-by: Claude Sonnet 5 <no...@anthropic.com>
(cherry picked from commit c060bee312bed71c52c23ac476ea28a70eba6ff2)
(cherry picked from commit 91bf9bad3d2d6579f729b8f9507064ce95f7901d)
---
.../SOLR-18324-jersey-null-request-npe.yml | 7 ++
.../apache/solr/handler/RequestHandlerBase.java | 5 +-
.../solr/jersey/CatchAllExceptionMapper.java | 6 +-
.../solr/jersey/PostRequestDecorationFilter.java | 4 +
.../solr/jersey/PostRequestLoggingFilter.java | 4 +-
.../apache/solr/jersey/RequestMetricHandling.java | 5 ++
.../solr/handler/RequestHandlerBaseTest.java | 21 ++++++
.../solr/jersey/RequestMetricHandlingTest.java | 87 ++++++++++++++++++++++
8 files changed, 135 insertions(+), 4 deletions(-)
diff --git a/changelog/unreleased/SOLR-18324-jersey-null-request-npe.yml b/changelog/unreleased/SOLR-18324-jersey-null-request-npe.yml
new file mode 100644
index 00000000000..e853f4442da
--- /dev/null
+++ b/changelog/unreleased/SOLR-18324-jersey-null-request-npe.yml
@@ -0,0 +1,7 @@
+title: The V2 JAX-RS API, when faced with an error, could throw an NPE masking the true problem.
+type: fixed
+authors:
+ - name: Eric Pugh
+links:
+ - name: SOLR-18324
+ url: https://issues.apache.org/jira/browse/SOLR-18324
diff --git a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
index e7322360ea5..5f59eae2f78 100644
--- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
@@ -321,9 +321,12 @@ public abstract class RequestHandlerBase
* <p>If a tragic exception occurred in the index writer, this method also gives up leadership of
* the shard, and replaces the index writer with a new one to attempt to get out of a transient
* failure (e.g. disk failure).
+ *
+ * <p>{@code req} may be null, e.g. when called from Jersey's {@code CatchAllExceptionMapper} for
+ * a request that failed before a {@link SolrQueryRequest} was attached to the request context.
*/
public static Exception processReceivedException(SolrQueryRequest req, Exception e) {
- SolrCore core = req.getCore();
+ SolrCore core = req == null ? null : req.getCore();
if (core != null) {
CoreContainer coreContainer = req.getCoreContainer();
assert coreContainer != null;
diff --git a/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java b/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java
index 506aa0670ac..8302235ecf8 100644
--- a/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java
+++ b/solr/core/src/java/org/apache/solr/jersey/CatchAllExceptionMapper.java
@@ -117,8 +117,10 @@ public class CatchAllExceptionMapper implements ExceptionMapper<Exception> {
shouldHideStackTrace(solrQueryRequest, containerRequestContext));
response.responseHeader.status = response.error.code;
final String mediaType =
- V2ApiUtils.getMediaTypeFromWtParam(
- solrQueryRequest.getParams(), MediaType.APPLICATION_JSON);
+ solrQueryRequest == null
+ ? MediaType.APPLICATION_JSON
+ : V2ApiUtils.getMediaTypeFromWtParam(
+ solrQueryRequest.getParams(), MediaType.APPLICATION_JSON);
return Response.status(response.error.code).type(mediaType).entity(response).build();
}
diff --git a/solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java b/solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java
index ea48aeba31d..5fb87de04af 100644
--- a/solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java
+++ b/solr/core/src/java/org/apache/solr/jersey/PostRequestDecorationFilter.java
@@ -55,6 +55,10 @@ public class PostRequestDecorationFilter implements ContainerResponseFilter {
}
final SolrQueryRequest solrQueryRequest =
(SolrQueryRequest) requestContext.getProperty(SOLR_QUERY_REQUEST);
+ if (solrQueryRequest == null) {
+ log.trace("Skipping QTime assignment because no SolrQueryRequest was attached");
+ return;
+ }
if (!responseContext.hasEntity()
|| !(responseContext.getEntity() instanceof SolrJerseyResponse)) {
log.debug("Skipping QTime assignment because response was not a SolrJerseyResponse");
diff --git a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
index b9bed2f8176..a6e798b97d0 100644
--- a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
+++ b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
@@ -86,7 +86,9 @@ public class PostRequestLoggingFilter implements ContainerResponseFilter {
final SolrQueryRequest solrQueryRequest =
(SolrQueryRequest) requestContext.getProperty(SOLR_QUERY_REQUEST);
final var solrConfig =
- (solrQueryRequest.getCore() != null) ? solrQueryRequest.getCore().getSolrConfig() : null;
+ (solrQueryRequest != null && solrQueryRequest.getCore() != null)
+ ? solrQueryRequest.getCore().getSolrConfig()
+ : null;
final Logger requestLogger = (solrConfig != null) ? coreRequestLogger : nonCoreRequestLogger;
final String templatedPath =
diff --git a/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java b/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java
index 533f56cd1a3..9dc4fd2934c 100644
--- a/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java
+++ b/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java
@@ -121,8 +121,13 @@ public class RequestMetricHandling {
} else {
log.debug("Skipping partialResults check because entity was not SolrJerseyResponse");
}
+ // Jersey can re-invoke response filters a second time when an exception occurs while
+ // building the first response (e.g. via CatchAllExceptionMapper), so guard against
+ // double-stopping the same timer.
final var timer = (AttributedLongTimer.MetricTimer) requestContext.getProperty(TIMER);
+ if (timer == null) return;
timer.stop();
+ requestContext.setProperty(TIMER, null);
}
}
}
diff --git a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java
index 137879f40e7..fe430c5907d 100644
--- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java
@@ -160,6 +160,27 @@ public class RequestHandlerBaseTest extends SolrTestCaseJ4 {
assertEquals(SolrException.ErrorCode.SERVER_ERROR.code, normalizedSolrException.code());
}
+ @Test
+ public void testNullRequestDoesNotThrowAndIsNotModified() {
+ // req can be null when this is called from Jersey's CatchAllExceptionMapper for a request
+ // that failed before a SolrQueryRequest was attached to the request context.
+ final Exception e = new RuntimeException("Some generic, non-SolrException");
+
+ final Exception normalized = RequestHandlerBase.processReceivedException(null, e);
+
+ assertSame(e, normalized);
+ }
+
+ @Test
+ public void testNullRequestWithSyntaxErrorIsStillWrappedIn400SolrException() {
+ final Exception e = new SyntaxError("Some syntax error");
+
+ final Exception normalized = RequestHandlerBase.processReceivedException(null, e);
+
+ assertEquals(SolrException.class, normalized.getClass());
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, ((SolrException) normalized).code());
+ }
+
@Test
public void testIsInternalShardRequest() {
final SolrQueryRequest solrQueryRequest =
diff --git a/solr/core/src/test/org/apache/solr/jersey/RequestMetricHandlingTest.java b/solr/core/src/test/org/apache/solr/jersey/RequestMetricHandlingTest.java
new file mode 100644
index 00000000000..df5bd6ac212
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/jersey/RequestMetricHandlingTest.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.jersey;
+
+import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito;
+import static org.apache.solr.jersey.RequestContextKeys.HANDLER_METRICS;
+import static org.apache.solr.jersey.RequestContextKeys.TIMER;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.metrics.LongCounter;
+import io.opentelemetry.api.metrics.LongHistogram;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerResponseContext;
+import java.util.Set;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Unit tests for {@link RequestMetricHandling} */
+public class RequestMetricHandlingTest extends SolrTestCase {
+
+ @BeforeClass
+ public static void ensureWorkingMockito() {
+ assumeWorkingMockito();
+ }
+
+ @Test
+ public void testPostRequestMetricsFilterToleratesBeingInvokedTwice() throws Exception {
+ // Jersey can re-invoke response filters a second time when an exception occurs while
+ // building the first response (e.g. via CatchAllExceptionMapper); the second invocation must
+ // not try to stop an already-stopped timer.
+ final var mockRequestContext = mock(ContainerRequestContext.class);
+ final var mockResponseContext = mock(ContainerResponseContext.class);
+ final RequestHandlerBase.HandlerMetrics metrics = createHandlerMetrics();
+ final var timer = metrics.requestTimes.start();
+
+ when(mockRequestContext.getPropertyNames()).thenReturn(Set.of());
+ when(mockRequestContext.getProperty(HANDLER_METRICS)).thenReturn(metrics);
+ when(mockRequestContext.getProperty(TIMER)).thenReturn(timer);
+
+ final var filter = new RequestMetricHandling.PostRequestMetricsFilter();
+ filter.filter(mockRequestContext, mockResponseContext);
+
+ // Simulate Jersey clearing the property after the first stop, as our fix does.
+ when(mockRequestContext.getProperty(TIMER)).thenReturn(null);
+
+ // A second invocation must not throw (would previously throw AssertionError with
+ // assertions enabled, from RTimer.stop() being called on an already-stopped timer).
+ filter.filter(mockRequestContext, mockResponseContext);
+ }
+
+ private RequestHandlerBase.HandlerMetrics createHandlerMetrics() {
+ final SolrMetricsContext metricsContext = mock(SolrMetricsContext.class);
+ final LongCounter mockLongCounter = mock(LongCounter.class);
+ final LongHistogram mockLongHistogram = mock(LongHistogram.class);
+
+ when(metricsContext.getRegistryName()).thenReturn("solr.core");
+ when(metricsContext.longCounter(any(), any())).thenReturn(mockLongCounter);
+ when(metricsContext.longCounter(any(), any(), any())).thenReturn(mockLongCounter);
+ when(metricsContext.longHistogram(any(), any())).thenReturn(mockLongHistogram);
+ when(metricsContext.longHistogram(any(), any(), any())).thenReturn(mockLongHistogram);
+
+ return new RequestHandlerBase.HandlerMetrics(
+ metricsContext, Attributes.of(AttributeKey.stringKey("source"), "test"), false);
+ }
+}