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

Consume all params and content on extension Rest Request exception #8096

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

dbwiddis
Copy link
Member

Description

RestRequest objects track the parameters and content, indented to provide users with more helpful error messages for incorrect API syntax.

When a Rest Request forwarded to an Extension fails, the onResponse() handler method never executes, and these "helpful" error messages end up giving the wrong reason for failure, hiding the actual exceptions.

This PR consumes all params and content to prevent this confusion, allowing the actual exception (often a timeout) to be sent to the user.

Related Issues

Resolves opensearch-project/opensearch-sdk-java#587

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Daniel Widdis <widdis@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #8096 (04ec510) into main (0b775e7) will decrease coverage by 0.16%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #8096      +/-   ##
============================================
- Coverage     70.92%   70.77%   -0.16%     
+ Complexity    56664    56507     -157     
============================================
  Files          4722     4722              
  Lines        267604   267606       +2     
  Branches      39214    39214              
============================================
- Hits         189803   189386     -417     
- Misses        61826    62219     +393     
- Partials      15975    16001      +26     
Impacted Files Coverage Δ
...rch/rest/extensions/RestSendToExtensionAction.java 46.66% <0.00%> (-0.80%) ⬇️

... and 455 files with indirect coverage changes

@owaiskazi19 owaiskazi19 merged commit 1f6a3bc into opensearch-project:main Jun 20, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 20, 2023
…8096)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
(cherry picked from commit 1f6a3bc)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
owaiskazi19 pushed a commit that referenced this pull request Jun 21, 2023
…8096) (#8183)

(cherry picked from commit 1f6a3bc)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gaiksaya pushed a commit to gaiksaya/OpenSearch that referenced this pull request Jun 21, 2023
gaiksaya pushed a commit to gaiksaya/OpenSearch that referenced this pull request Jun 26, 2023
…pensearch-project#8096) (opensearch-project#8183)

(cherry picked from commit 1f6a3bc)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request Jun 27, 2023
…pensearch-project#8096)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Rishab Nahata <rnnahata@amazon.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…pensearch-project#8096)

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Unconsumed params error message hides Extension REST request timeouts
3 participants