converting illegalstate exception to opensearch status exception with 400 status

[dhrubo-os]

converting illegalstate exception to opensearch status exception with 400 status

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.

Summary by CodeRabbit

• Bug Fixes

  • Standardized error responses for model deploy/predict/register and connector/stream/update when local or remote inference is disabled — callers now receive REST-style errors with HTTP 400 (Bad Request).

• Tests

  • Updated tests to expect the new REST-style exceptions and to assert the HTTP 400 status for disabled local/remote inference scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Replaced thrown IllegalStateException with OpenSearchStatusException(..., RestStatus.BAD_REQUEST) across ML action handlers, REST endpoints (including connector and stream handlers), and corresponding tests; added necessary imports and updated test assertions for exception status.

Changes

Cohort / File(s)	Summary
Action Handlers plugin/src/main/java/org/opensearch/ml/action/deploy/TransportDeployModelAction.java, plugin/src/main/java/org/opensearch/ml/action/prediction/TransportPredictionTaskAction.java, plugin/src/main/java/org/opensearch/ml/action/register/TransportRegisterModelAction.java	Replaced IllegalStateException(...) with OpenSearchStatusException(..., RestStatus.BAD_REQUEST) for local/remote model-feature-disabled conditions; added imports for OpenSearchStatusException and RestStatus.
REST Handlers (prediction/register) plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java, plugin/src/main/java/org/opensearch/ml/rest/RestMLRegisterModelAction.java	Replaced IllegalStateException with OpenSearchStatusException(..., RestStatus.BAD_REQUEST) in request validation paths; added imports.
REST Handlers (connector/stream/update) plugin/src/main/java/org/opensearch/ml/rest/RestMLCreateConnectorAction.java, plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionStreamAction.java, plugin/src/main/java/org/opensearch/ml/rest/RestMLUpdateConnectorAction.java	Replaced IllegalStateException with OpenSearchStatusException(..., RestStatus.BAD_REQUEST) when remote inference is disabled; added imports.
Action Tests plugin/src/test/java/org/opensearch/ml/action/deploy/TransportDeployModelActionTests.java, plugin/src/test/java/org/opensearch/ml/action/prediction/TransportPredictionTaskActionTests.java, plugin/src/test/java/org/opensearch/ml/action/register/TransportRegisterModelActionTests.java	Updated tests to expect OpenSearchStatusException and assert status() == RestStatus.BAD_REQUEST; adjusted imports.
REST Handler Tests (prediction/register) plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java, plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionActionTests.java	Updated tests to expect OpenSearchStatusException and verify RestStatus.BAD_REQUEST; added imports.
REST Connector & Stream Tests plugin/src/test/java/org/opensearch/ml/rest/RestMLCreateConnectorActionTests.java, plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionStreamActionTests.java, plugin/src/test/java/org/opensearch/ml/rest/RestMLUpdateConnectorActionTests.java	Updated tests to expect OpenSearchStatusException for feature-disabled paths and assert RestStatus.BAD_REQUEST where applicable; imported OpenSearchStatusException / RestStatus.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify all replaced throw sites use OpenSearchStatusException(..., RestStatus.BAD_REQUEST) with identical messages/constants.
• Ensure added imports are used and no unused imports remain.
• Confirm unit tests check both exception type and status() and that no control flow behavior changed beyond exception type/status.

Poem

A rabbit hops through code at night,
Swapping fright for HTTP light,
Four-oh-oh now greets the case,
Tests nod, all in proper place,
🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is incomplete. The 'Description' section only repeats the title without explaining what the change achieves, and the 'Related Issues' section contains an unfilled placeholder without an actual issue number.	Add a meaningful description explaining the purpose and benefits of replacing IllegalStateException with OpenSearchStatusException, and replace the placeholder with an actual issue number or leave it blank if no issue exists.
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.79% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly reflects the main change: converting IllegalStateException to OpenSearchStatusException with HTTP 400 status, which aligns with all file changes in the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ddd42d and 6204719.

📒 Files selected for processing (1)

• plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionActionTests.java (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionActionTests.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: spotless

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java (1)

147-156: Inconsistent exception handling across feature-disabled scenarios.

While line 151 correctly uses OpenSearchStatusException with RestStatus.BAD_REQUEST for LOCAL_MODEL_DISABLED_ERR_MSG, lines 148 and 153 still throw IllegalStateException for REMOTE_INFERENCE_DISABLED_ERR_MSG and BATCH_INFERENCE_DISABLED_ERR_MSG. This creates inconsistent error handling for similar "feature disabled" scenarios.

For consistent REST semantics, consider applying the same exception type and status code to all feature-disabled checks.

 if (FunctionName.REMOTE.name().equals(modelType) && !mlFeatureEnabledSetting.isRemoteInferenceEnabled()) {
-    throw new IllegalStateException(REMOTE_INFERENCE_DISABLED_ERR_MSG);
+    throw new OpenSearchStatusException(REMOTE_INFERENCE_DISABLED_ERR_MSG, RestStatus.BAD_REQUEST);
 } else if (FunctionName.isDLModel(FunctionName.from(modelType.toUpperCase(Locale.ROOT)))
     && !mlFeatureEnabledSetting.isLocalModelEnabled()) {
     throw new OpenSearchStatusException(LOCAL_MODEL_DISABLED_ERR_MSG, RestStatus.BAD_REQUEST);
 } else if (ActionType.BATCH_PREDICT == actionType && !mlFeatureEnabledSetting.isOfflineBatchInferenceEnabled()) {
-    throw new IllegalStateException(BATCH_INFERENCE_DISABLED_ERR_MSG);
+    throw new OpenSearchStatusException(BATCH_INFERENCE_DISABLED_ERR_MSG, RestStatus.BAD_REQUEST);
 } else if (!ActionType.isValidActionInModelPrediction(actionType)) {
     throw new IllegalArgumentException("Wrong action type in the rest request path!");
 }

🧹 Nitpick comments (1)

plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java (1)

167-173: Updated exception expectation aligns with new local‑model error behavior; consider also asserting status

Switching the expectation from IllegalStateException to OpenSearchStatusException for the local‑inference‑disabled path matches the new behavior while keeping the message assertion intact. If you want this test to also guard the HTTP mapping, you could switch to assertThrows and additionally check e.status() for RestStatus.BAD_REQUEST, but that’s optional and can be deferred.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 974124c and 7a3aea9.

📒 Files selected for processing (9)

• plugin/src/main/java/org/opensearch/ml/action/deploy/TransportDeployModelAction.java (1 hunks)
• plugin/src/main/java/org/opensearch/ml/action/prediction/TransportPredictionTaskAction.java (1 hunks)
• plugin/src/main/java/org/opensearch/ml/action/register/TransportRegisterModelAction.java (3 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java (2 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLRegisterModelAction.java (2 hunks)
• plugin/src/test/java/org/opensearch/ml/action/deploy/TransportDeployModelActionTests.java (2 hunks)
• plugin/src/test/java/org/opensearch/ml/action/prediction/TransportPredictionTaskActionTests.java (1 hunks)
• plugin/src/test/java/org/opensearch/ml/action/register/TransportRegisterModelActionTests.java (3 hunks)
• plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and Test MLCommons Plugin on linux (25)
• GitHub Check: Build and Test MLCommons Plugin on linux (21)
• GitHub Check: Build and Test MLCommons Plugin on Windows (25)
• GitHub Check: Build and Test MLCommons Plugin on Windows (21)

🔇 Additional comments (6)

plugin/src/test/java/org/opensearch/ml/action/prediction/TransportPredictionTaskActionTests.java (1)

194-202: LGTM! Test correctly validates exception type and status.

The test properly verifies both the exception type (OpenSearchStatusException) and the HTTP status code (RestStatus.BAD_REQUEST), aligning with the production code changes.

plugin/src/test/java/org/opensearch/ml/action/register/TransportRegisterModelActionTests.java (1)

281-289: LGTM! Test correctly updated for new exception semantics.

The test appropriately expects OpenSearchStatusException and validates the RestStatus.BAD_REQUEST status code, consistent with the production code changes.

plugin/src/test/java/org/opensearch/ml/action/deploy/TransportDeployModelActionTests.java (1)

400-403: LGTM! Test correctly validates new exception semantics.

The test appropriately expects OpenSearchStatusException and asserts the RestStatus.BAD_REQUEST status, aligning with the production code changes.

plugin/src/main/java/org/opensearch/ml/action/prediction/TransportPredictionTaskAction.java (1)

131-133: LGTM! Correct exception type for REST-aware error handling.

The change appropriately replaces IllegalStateException with OpenSearchStatusException carrying RestStatus.BAD_REQUEST, providing proper HTTP semantics for client error handling.

plugin/src/main/java/org/opensearch/ml/action/register/TransportRegisterModelAction.java (1)

169-171: LGTM! Correct exception type for feature validation.

The change appropriately replaces IllegalStateException with OpenSearchStatusException carrying RestStatus.BAD_REQUEST, providing proper HTTP status semantics for disabled feature scenarios.

plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java (1)

30-30: Import of OpenSearchStatusException is appropriate and correctly used

The added import is needed for the updated test expectation below and is used without introducing any unused-import noise. No changes requested here.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionStreamAction.java (1)

137-139: Consider applying the same pattern for consistency.

Line 138 still throws IllegalStateException when streaming is disabled, while Line 323 now throws OpenSearchStatusException with BAD_REQUEST for disabled remote inference. Both represent "feature disabled" scenarios. For consistency, consider converting this to OpenSearchStatusException as well.

Apply this diff for consistency:

 if (!mlFeatureEnabledSetting.isStreamEnabled()) {
-    throw new IllegalStateException(STREAM_DISABLED_ERR_MSG);
+    throw new OpenSearchStatusException(STREAM_DISABLED_ERR_MSG, RestStatus.BAD_REQUEST);
 }

plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java (1)

152-153: Consider applying the same pattern for consistency.

Line 153 still throws IllegalStateException when batch inference is disabled, while Lines 148 and 151 now throw OpenSearchStatusException with BAD_REQUEST. All three represent "feature disabled" scenarios. For consistency, consider converting this to OpenSearchStatusException as well.

Apply this diff for consistency:

 } else if (ActionType.BATCH_PREDICT == actionType && !mlFeatureEnabledSetting.isOfflineBatchInferenceEnabled()) {
-    throw new IllegalStateException(BATCH_INFERENCE_DISABLED_ERR_MSG);
+    throw new OpenSearchStatusException(BATCH_INFERENCE_DISABLED_ERR_MSG, RestStatus.BAD_REQUEST);
 } else if (!ActionType.isValidActionInModelPrediction(actionType)) {

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4612118 and 1ddd42d.

📒 Files selected for processing (11)

• plugin/src/main/java/org/opensearch/ml/action/deploy/TransportDeployModelAction.java (1 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLCreateConnectorAction.java (2 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java (2 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionStreamAction.java (1 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLRegisterModelAction.java (2 hunks)
• plugin/src/main/java/org/opensearch/ml/rest/RestMLUpdateConnectorAction.java (2 hunks)
• plugin/src/test/java/org/opensearch/ml/action/deploy/TransportDeployModelActionTests.java (3 hunks)
• plugin/src/test/java/org/opensearch/ml/rest/RestMLCreateConnectorActionTests.java (2 hunks)
• plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionStreamActionTests.java (2 hunks)
• plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java (3 hunks)
• plugin/src/test/java/org/opensearch/ml/rest/RestMLUpdateConnectorActionTests.java (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• plugin/src/main/java/org/opensearch/ml/rest/RestMLRegisterModelAction.java
• plugin/src/test/java/org/opensearch/ml/rest/RestMLRegisterModelActionTests.java
• plugin/src/main/java/org/opensearch/ml/action/deploy/TransportDeployModelAction.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: spotless

🔇 Additional comments (9)

plugin/src/main/java/org/opensearch/ml/rest/RestMLUpdateConnectorAction.java (1)

21-22: LGTM - Exception handling improved for REST semantics.

The change from IllegalStateException to OpenSearchStatusException with RestStatus.BAD_REQUEST properly signals a 400 status to clients when remote inference is disabled, which is more appropriate than the generic 500 error that IllegalStateException would produce.

Also applies to: 65-65

plugin/src/test/java/org/opensearch/ml/rest/RestMLUpdateConnectorActionTests.java (1)

28-28: LGTM - Test updated to match the new exception type.

The test correctly expects OpenSearchStatusException instead of IllegalStateException, maintaining consistency with the production code changes.

Also applies to: 140-140

plugin/src/test/java/org/opensearch/ml/action/deploy/TransportDeployModelActionTests.java (2)

371-387: LGTM - Comprehensive test coverage for remote inference disabled scenario.

The test correctly verifies both the exception type (OpenSearchStatusException) and the HTTP status (RestStatus.BAD_REQUEST), ensuring the error response is properly structured.

---

389-405: LGTM - Comprehensive test coverage for local inference disabled scenario.

The test follows the same pattern as the remote inference test, properly validating both exception type and HTTP status code.

plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionStreamAction.java (1)

323-323: LGTM - Consistent exception handling for disabled remote inference.

The change aligns with the PR objective and matches the pattern used across other REST actions.

plugin/src/test/java/org/opensearch/ml/rest/RestMLCreateConnectorActionTests.java (1)

30-30: LGTM - Test aligned with production code changes.

The test expectation correctly reflects the new exception type for disabled remote inference.

Also applies to: 135-135

plugin/src/test/java/org/opensearch/ml/rest/RestMLPredictionStreamActionTests.java (1)

161-173: LGTM - Thorough validation of exception type and HTTP status.

The test properly verifies both the OpenSearchStatusException type and the RestStatus.BAD_REQUEST status, ensuring the API returns the correct HTTP semantics.

plugin/src/main/java/org/opensearch/ml/rest/RestMLPredictionAction.java (1)

147-151: LGTM - Exception handling improved for both remote and local model scenarios.

Both disabled inference checks now properly return HTTP 400 status via OpenSearchStatusException, improving REST API semantics.

plugin/src/main/java/org/opensearch/ml/rest/RestMLCreateConnectorAction.java (1)

18-19: LGTM - Proper REST error handling for disabled remote inference.

The change to OpenSearchStatusException with RestStatus.BAD_REQUEST ensures clients receive an appropriate HTTP 400 response instead of a generic server error.

Also applies to: 71-71

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.15%. Comparing base (c243f8a) to head (6204719).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #4484   +/-   ##
=========================================
  Coverage     80.15%   80.15%           
- Complexity    10227    10238   +11     
=========================================
  Files           858      858           
  Lines         44496    44517   +21     
  Branches       5145     5151    +6     
=========================================
+ Hits          35664    35683   +19     
- Misses         6668     6670    +2     
  Partials       2164     2164           

Flag	Coverage Δ
ml-commons	80.15% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.