You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "Paul-Blanchaert (via GitHub)" <gi...@apache.org> on 2024/12/23 20:58:06 UTC
[PR] SOLR-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Paul-Blanchaert opened a new pull request, #2926:
URL: https://github.com/apache/solr/pull/2926
[https://issues.apache.org/jira/browse/SOLR-17589](https://issues.apache.org/jira/browse/SOLR-17589)
# Description
Adding header "Content-Type"="application/json" to HEAD request prevents that the server side raises the SolrException.
# Solution
Added header "Content-Type"="application/json" to HEAD request
# Tests
Existing unit tests show the issue in the test log (and ignore the server side SolrException).
Re-running the tests with this commit don't show the issue.
Adding a new integration test (client side changes raises server side exception) seems too cumbersome for this change where positive result is demonstrated in the test log and sufficient existing unit test validate that the functionality works fine. Furthermore, it seems to me that adding a (formerly missing) valid http header to a client side http request should not raise issues that are not covered by current test set. When that specific header fails, it should fail fast.
# Checklist
Please review the following and check all that apply:
- [X] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [X] I have created a Jira issue and added the issue ID to my pull request title.
- [X] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
- [X] I have developed this patch against the `main` branch.
- [X] I have run `./gradlew check` (with -Pvalidation.rat.failOnError=false).
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "jdyer1 (via GitHub)" <gi...@apache.org>.
jdyer1 merged PR #2926:
URL: https://github.com/apache/solr/pull/2926
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "jdyer1 (via GitHub)" <gi...@apache.org>.
jdyer1 commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2583458199
@Paul-Blanchaert This is great, although I am surprised the host jetty behaves such.
Can you both sync this branch to `main` and add a CHANGES.txt entry under solr 9.9 (be sure to give yourself credit)? Otherwise, if you give access to push to your branch, I will take care of those details.
Once the CHANGES.txt entry is there, we can go ahead and merge & backport for 9.9.
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "Paul-Blanchaert (via GitHub)" <gi...@apache.org>.
Paul-Blanchaert commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2565544255
The initial problem was solved by adding the http header "Content-Type" while keeping the noBody() (see below), so no need to store the body in a separate variable.
HttpRequest.newBuilder(uriNoQueryParams)
.method("HEAD", HttpRequest.BodyPublishers.**noBody**())
.header("**Content-Type**", ClientUtils.TEXT_JSON);
I reverted the change and test on the http status.
And I followed your suggestion by assuring that the header "Content-Type" is set via the DebugServlet. This is basically just testing whether the header function on the builder works fine, but It might prevent that future changes omit the content-type header that is apparently required to avoid the server exception on the HEAD request.
Hope this helps!
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "Paul-Blanchaert (via GitHub)" <gi...@apache.org>.
Paul-Blanchaert commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2563580128
Thanks for the encouragement.
Can you help me understand what you're asking with "an assert somewhere"?
In order to be able to test the solr server exception, I'll need to change the scope of the function to "collect" the http statuscode (400 is returned in the case of the solr server exception). Currently, the http response (and status) is ignored (headSucceeded = true when no exception is thrown by the jdk http client; this probably goes for any 4xx/5xx statusses; but I didn't verify that). Should I change the function to set headSucceeded to false when http statuscode <> 200?
That way, we're able to capture the exception prior to the change of this PR and we can build a unit test to validate the "enhanced" function.
Is that what you had in mind?
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "Paul-Blanchaert (via GitHub)" <gi...@apache.org>.
Paul-Blanchaert commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2586528636
@jdyer1
I've added the changes.txt entry but I'm probably not able to close the PR. Can you take care of remaining tasks?
I should have created the fork under my user (not the organisation), but apparently I didn't. So I've granted you write access to the fork.
Let me know if I can help with anything else...
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "jdyer1 (via GitHub)" <gi...@apache.org>.
jdyer1 commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2563894874
Here is an approach that won't need any change to the client itself.
1. See `DebugServlet.java' method 'doHead'. Here it records the request body in a variable named `requestBody`, but unfortunately any subsequent POST request will overwrite it before the test can examine it. `DebugServlet` will need to keep the HEAD request body in a separate variable.
2. See `HttpJdkSolrClientTest.java' method `assertNoHeadRequestWithSsl`. Here it asserts that if the randomized test runner configured an 'https' connection, then the HEAD request should not be fired. We can additionally assert that in the other case, whenever the HEAD request is fired, that it contains a body, by looking at the new variable created in `DebugServlet`.
--
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-17589 First use of PUT/POST request with HttpJdkSolrClient generates error log entry on solr server due to initial HEAD request [solr]
Posted by "jdyer1 (via GitHub)" <gi...@apache.org>.
jdyer1 commented on PR #2926:
URL: https://github.com/apache/solr/pull/2926#issuecomment-2560434916
This is good. Is there any possibility you can add an assert somewhere to `HttpJdkSolrClientTest` to test the change?
--
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