[Agentic Search] Forward OpenSearchStatusException from agent execution

[owaiskazi19]

Description

Forward OpenSearchStatusException from agent execution to preserve error codes

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 895d4a9)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Missing Test Coverage The getAgentDetails failure path (second error handler) that now forwards OpenSearchStatusException is not covered by a test. Only the executeAgent failure path has a new test (testProcessRequestAsync_withAgenticQuery_agentThrottled_forwardsStatus). A similar test for the agent info retrieval failure with an OpenSearchStatusException should be added. if (e instanceof OpenSearchStatusException) { requestListener.onFailure(e); } else { requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); } Duplicate Logic The same instanceof OpenSearchStatusException check and forwarding logic is duplicated in two separate error handlers. Consider extracting this into a helper method (e.g., handleAgentFailure(Exception e, String errorMessage, ActionListener<?> listener)) to reduce duplication and ease future maintenance. if (e instanceof OpenSearchStatusException) { requestListener.onFailure(e); } else { requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); } }) ); }, e -> { String errorMessage = String.format( Locale.ROOT, "Failed to get agent info - Agent ID: [%s], Error: [%s]", agentId, e.getMessage() ); agenticQuery.setAgentFailureReason(errorMessage); if (e instanceof OpenSearchStatusException) { requestListener.onFailure(e); } else { requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 895d4a9
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Preserve context when forwarding status exceptions When forwarding an OpenSearchStatusException directly, the agenticQuery.setAgentFailureReason(errorMessage) is still called with the formatted error message, but the original exception is passed without wrapping. Consider also wrapping the OpenSearchStatusException with additional context (or at minimum ensure the failure reason is consistent), so that the error message stored in agenticQuery matches what is propagated to the caller. Alternatively, extract a helper method to avoid duplicating this logic in both error handlers. src/main/java/org/opensearch/neuralsearch/processor/AgenticQueryTranslatorProcessor.java [229-233] agenticQuery.setAgentFailureReason(errorMessage); if (e instanceof OpenSearchStatusException) { - requestListener.onFailure(e); + OpenSearchStatusException statusException = (OpenSearchStatusException) e; + requestListener.onFailure(new OpenSearchStatusException("Agentic search failed - " + errorMessage, statusException.status(), statusException)); } else { requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); } Suggestion importance[1-10]: 4 __ Why: The suggestion proposes wrapping OpenSearchStatusException with additional context, but the PR's explicit intent is to preserve the original status exception (including its HTTP status code) by forwarding it directly. Wrapping it in a new OpenSearchStatusException adds some context but changes the behavior slightly; the trade-off is debatable and the improvement is minor. The duplicate logic concern is valid but low-impact.	Low

---

Previous suggestions

Suggestions up to commit 895d4a9

Category	Suggestion	Impact
General	Preserve contextual error message when forwarding status exceptions When forwarding the OpenSearchStatusException directly, the errorMessage containing the agent ID and original error details is discarded. Callers receiving the raw exception will lose the contextual information about which agent failed. Consider wrapping the OpenSearchStatusException to preserve both the HTTP status and the contextual error message, or at minimum logging the errorMessage before forwarding. src/main/java/org/opensearch/neuralsearch/processor/AgenticQueryTranslatorProcessor.java [229-233] if (e instanceof OpenSearchStatusException) { - requestListener.onFailure(e); + OpenSearchStatusException statusException = (OpenSearchStatusException) e; + requestListener.onFailure(new OpenSearchStatusException(errorMessage, statusException.status(), statusException)); } else { requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); } Suggestion importance[1-10]: 6 __ Why: This is a valid concern — forwarding the raw OpenSearchStatusException discards the errorMessage context (agent ID, etc.) that was carefully constructed. Wrapping it with the contextual message while preserving the HTTP status code would provide better debugging information to callers.	Low
	Extract duplicate error-forwarding logic into helper method The agenticQuery.setAgentFailureReason(errorMessage) is called before the check, but when forwarding the OpenSearchStatusException directly, the errorMessage (which includes the agent ID context) is set on the query object but not included in the forwarded exception. Consider wrapping the OpenSearchStatusException with additional context or at least ensuring the error message is consistent. Additionally, using instanceof pattern matching (Java 16+) or casting to a common exception type would be cleaner, but more critically, the same duplicate logic block appears twice — consider extracting it into a helper method to avoid divergence. src/main/java/org/opensearch/neuralsearch/processor/AgenticQueryTranslatorProcessor.java [229-233] -if (e instanceof OpenSearchStatusException) { - requestListener.onFailure(e); -} else { - requestListener.onFailure(new IllegalArgumentException("Agentic search failed - " + errorMessage, e)); -} +agenticQuery.setAgentFailureReason(errorMessage); +forwardOrWrapFailure(requestListener, e, errorMessage); Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies duplicate logic in two places, but the improved_code only shows calling a helper method without providing the actual implementation, making it incomplete. The refactoring would improve maintainability but is a minor style improvement.	Low

[github-actions]

Persistent review updated to latest commit 895d4a9

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.05%. Comparing base (af3a5cf) to head (895d4a9).

Files with missing lines	Patch %	Lines
...rch/processor/AgenticQueryTranslatorProcessor.java	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1851      +/-   ##
============================================
- Coverage     82.05%   82.05%   -0.01%     
- Complexity     3763     3765       +2     
============================================
  Files           289      289              
  Lines         13659    13663       +4     
  Branches       2236     2238       +2     
============================================
+ Hits          11208    11211       +3     
- Misses         1632     1633       +1     
  Partials        819      819              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[yuye-aws]

The code looks good to me. Can you resolve the conflicts?