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

Fix returns 500 error for AI agent APIs when OpenSearch returns 4xx error with json format error message #380

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

gaobinlong
Copy link
Collaborator

@gaobinlong gaobinlong commented Dec 26, 2024

Description

In this PR:#373, we returns 4xx error for the AI agent APIs when the execute agent API in ml-commons plugin responds with a 4xx error, but previously, the execute agent API in ml-commons plugin returns string format error message if the request failed, now it changed to json format, this triggers different condition here in OSD-core, so we the AI agent APIs return 500 internal error because the custom response body doesn't have a message field, this PR fixed the bug.

In addition, when customizing the response, headers is not needed because the headers comes from ml-commons API, that is useless for node APIs.

image image

Issues Resolved

No issue.

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test.
  • New functionality has user manual doc added.
  • Commits are signed per the DCO using --signoff.

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.

…ode with json format error message

Signed-off-by: gaobinlong <[email protected]>
Signed-off-by: gaobinlong <[email protected]>
@gaobinlong gaobinlong added backport 2.x Trigger the backport flow to 2.x v2.19.0 labels Dec 26, 2024
Copy link
Member

@ruanyl ruanyl left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for adding the tests

statusCode: e.statusCode,
headers: e.headers,
});
} else {
return res.customError({
body: 'Execute agent failed!',
statusCode: 500,
Copy link
Member

Choose a reason for hiding this comment

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

Should we return original statusCode? As long as we don't disclose server side information, we should be fine.

@ruanyl
Copy link
Member

ruanyl commented Dec 27, 2024

I will merge now, will address @xluo-aws concern in a separate PR

@ruanyl ruanyl merged commit ffa509b into opensearch-project:main Dec 27, 2024
14 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 27, 2024
…rror with json format error message (#380)

* Fix returns 500 error for AI agent APIs when OpenSearch returns 4xx code with json format error message

Signed-off-by: gaobinlong <[email protected]>

* Modify change log

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
(cherry picked from commit ffa509b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
@gaobinlong
Copy link
Collaborator Author

I will merge now, will address @xluo-aws concern in a separate PR

@xluo-aws @ruanyl OK, I'll do some test to verify that return the original 5xx status code doesn't introduce some unexpected behavior.

ruanyl pushed a commit that referenced this pull request Dec 27, 2024
…rror with json format error message (#380) (#381)

* Fix returns 500 error for AI agent APIs when OpenSearch returns 4xx code with json format error message

Signed-off-by: gaobinlong <[email protected]>

* Modify change log

Signed-off-by: gaobinlong <[email protected]>

---------

Signed-off-by: gaobinlong <[email protected]>
(cherry picked from commit ffa509b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Trigger the backport flow to 2.x v2.19.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants