Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17209: Fix NullPointerException in QueryComponent #2354

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

pvcnt
Copy link
Contributor

@pvcnt pvcnt commented Mar 19, 2024

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

Description

In our production deployment of Solr, running 8.6, we see the following stacktrace (truncated):

java.lang.NullPointerException at org.apache.solr.handler.component.QueryComponent.mergeIds(QueryComponent.java:873) at org.apache.solr.handler.component.QueryComponent.handleRegularResponses(QueryComponent.java:628) at org.apache.solr.handler.component.QueryComponent.handleResponses(QueryComponent.java:607)

Solution

Add a null check.

Tests

None.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@epugh
Copy link
Contributor

epugh commented Mar 19, 2024

I'm hoping someone else with more experience can weigh in.... However, isn't the bug here that you can have a SolrServerException that doesn't have a cause? Shouldn't we just make sure we always have a cause? I'd love to see a unit test demonstrating where this happens normally, and therefore why the extra check is needed?

@pvcnt
Copy link
Contributor Author

pvcnt commented Mar 19, 2024

I have observed it in production (running on 8.6, granted). SolrServerException sure has a constructor that takes no cause, and is used at several places in Http2SolrClient, HttpSolrClient and LBSolrClient. I guess the extra check does not hurt, since the code definitely allows the cause to be null?

Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eric: Exceptions are chained; the end of the chain has no cause. Any code that throws SSE directly (not due to some other exception) will result in this.

I know it's annoying but can you add a CHANGES.txt item and file a JIRA please?

@pvcnt pvcnt changed the title Fix NPE in QueryComponent Fix NullPointerException in QueryComponent Mar 20, 2024
@pvcnt
Copy link
Contributor Author

pvcnt commented Mar 20, 2024

@dsmiley I thought I could do the economy of it for such a small change. I created the issue and entry in CHANGES.txt.

@dsmiley dsmiley changed the title Fix NullPointerException in QueryComponent SOLR-17209 Fix NullPointerException in QueryComponent Mar 22, 2024
@dsmiley dsmiley changed the title SOLR-17209 Fix NullPointerException in QueryComponent SOLR-17209: Fix NullPointerException in QueryComponent Mar 22, 2024
@epugh
Copy link
Contributor

epugh commented Mar 22, 2024

@dsmiley do you want to shepherd this one? If not, I will, just let me know ;-). And thanks for explaining about the exceptions!

@dsmiley
Copy link
Contributor

dsmiley commented Mar 22, 2024

It'd be wonderful for you to; thanks Eric. Separately, I'm trying to combat the need for JIRAs; I hated asking for one here. Stay tuned.

@epugh epugh self-assigned this Mar 22, 2024
@epugh epugh merged commit 1a322d9 into apache:main Mar 25, 2024
2 of 3 checks passed
epugh added a commit that referenced this pull request Mar 25, 2024
---------

Co-authored-by: Vincent Primault <vprimault@salesforce.com>
Co-authored-by: Eric Pugh <epugh@opensourceconnections.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants