Skip to content

Conversation

@karenyrx
Copy link
Contributor

@karenyrx karenyrx commented Aug 5, 2025

Description

Improve transport-grpc error handling, to use proper GRPC conventions, but also maintain parity with HTTP APIs.

  1. Replace HTTP status codes in responses with gRPC ones: Previously, returned 2xx, 4xx, 5xx in the protobuf responses, but now return GRPC codes (0, 1, 2)
  2. Proper gRPC error codes: Previously, all GRPC errors were mapped to the GRPC.INTERNAL code. Now returns the correct fine-grained status codes (INVALID_ARGUMENT, RESOURCE_EXHAUSTED, etc.)
  3. Exact Parity of error messages: Both APIs now use constants and shared methods to provide identical error messages, including use the correct wrapping.
  4. Graceful Fallback: Uses appropriate fallback messages for null/empty exception messages (parity with HTTP as well)

Detailed Examples

1. Remove HTTP status codes from protobuf messages

  • Before: HTTP status codes in protobuf responses
ResponseItem {
  status: 201  // HTTP Created status code
}
  • After: gRPC status codes in protobuf responses
ResponseItem {
  status: 0    // gRPC OK status code
}

2. Return proper GRPC status codes instead of all showing as GRPC.INTERNAL

  • Before: Inconsistent status codes
OpenSearchRejectedExecutionException -> INTERNAL (status code 13)
CircuitBreakingException -> INTERNAL (status code 13)
// HTTP returned 429 TOO_MANY_REQUESTS, gRPC returned INTERNAL
  • After: Proper resource exhaustion status
OpenSearchRejectedExecutionException -> RESOURCE_EXHAUSTED (status code 8)
CircuitBreakingException -> RESOURCE_EXHAUSTED (status code 8)
// Message: "Too many requests" (consistent with HTTP 429)

3. Use constants for error messages so both HTTP and gRPC now use identical error messages from ExceptionsHelper.ErrorMessages

  • Before: Different error messages between HTTP and gRPC
// HTTP API returned: "Invalid argument"
// gRPC API returned: "Invalid parameter" (inconsistent)
  • After: Identical error messages using shared constants
// Both HTTP and gRPC return: "Invalid argument"
// (using ExceptionsHelper.ErrorMessages.INVALID_ARGUMENT)

4. Wrap error messages in the same way as HTTP

  • Before: Different error formats
// HTTP API: "OpenSearchException[Index not found]"
// gRPC API: "Index not found" (lost exception type context)
  • After: Identical error format using ExceptionsHelper.summaryMessage()
// Both APIs: "OpenSearchException[Index not found]"

Related Issues

Resolves #18926

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

@karenyrx karenyrx marked this pull request as ready for review August 5, 2025 16:46
@karenyrx karenyrx requested a review from a team as a code owner August 5, 2025 16:46
@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request Libraries Lucene Upgrades and Libraries, Any 3rd party library that Core depends on, ex: nebula; team is respo labels Aug 5, 2025
@karenyrx karenyrx moved this to In Progress in gRPC/Protobuf Aug 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for 159a844: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for 945bb64: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

❌ Gradle check result for dd84c75: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

✅ Gradle check result for 352aaa1: SUCCESS

@codecov
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

❌ Patch coverage is 88.50575% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.90%. Comparing base (696ed65) to head (5dcf5c7).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...transport/grpc/util/RestToGrpcStatusConverter.java 82.60% 3 Missing and 1 partial ⚠️
...rt/grpc/listeners/SearchRequestActionListener.java 50.00% 3 Missing ⚠️
...ensearch/transport/grpc/util/GrpcErrorHandler.java 93.54% 1 Missing and 1 partial ⚠️
...src/main/java/org/opensearch/ExceptionsHelper.java 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18925      +/-   ##
============================================
+ Coverage     72.88%   72.90%   +0.02%     
- Complexity    69300    69333      +33     
============================================
  Files          5642     5644       +2     
  Lines        318640   318707      +67     
  Branches      46108    46119      +11     
============================================
+ Hits         232237   232355     +118     
+ Misses        67566    67525      -41     
+ Partials      18837    18827      -10     

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

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❌ Gradle check result for 6b93cb6: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@karenyrx karenyrx closed this Aug 6, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done/Won't Do in gRPC/Protobuf Aug 6, 2025
@karenyrx karenyrx reopened this Aug 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❌ Gradle check result for 6b93cb6: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

✅ Gradle check result for 859e0fa: SUCCESS

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

❌ Gradle check result for 5ff1ad2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

❌ Gradle check result for 5dcf5c7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

✅ Gradle check result for 5dcf5c7: SUCCESS

@andrross andrross merged commit be8bbe1 into opensearch-project:main Aug 7, 2025
58 of 60 checks passed
@andrross andrross added the backport 3.2 Backport to 3.2 branch label Aug 8, 2025
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 8, 2025
… parity with HTTP (#18925)

* [GRPC] Add proper GRPC status code and error handling

Signed-off-by: Karen Xu <[email protected]>

* rename Http to Rest, group together some convertRestToGrpcStatus codes

Signed-off-by: Karen Xu <[email protected]>

* make final

Signed-off-by: Karen Xu <[email protected]>

---------

Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen X <[email protected]>
(cherry picked from commit be8bbe1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
andrross added a commit that referenced this pull request Aug 8, 2025
…eption handling parity with HTTP (#18990)

* [GRPC] Map to proper GRPC status codes and achieve exception handling parity with HTTP (#18925)

* [GRPC] Add proper GRPC status code and error handling

Signed-off-by: Karen Xu <[email protected]>

* rename Http to Rest, group together some convertRestToGrpcStatus codes

Signed-off-by: Karen Xu <[email protected]>

* make final

Signed-off-by: Karen Xu <[email protected]>

---------

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

* Add entry to release notes

Signed-off-by: Andrew Ross <[email protected]>

---------

Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen X <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Andrew Ross <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andrew Ross <[email protected]>
RajatGupta02 pushed a commit to RajatGupta02/OpenSearch that referenced this pull request Aug 18, 2025
… parity with HTTP (opensearch-project#18925)

* [GRPC] Add proper GRPC status code and error handling

Signed-off-by: Karen Xu <[email protected]>

* rename Http to Rest, group together some convertRestToGrpcStatus codes

Signed-off-by: Karen Xu <[email protected]>

* make final

Signed-off-by: Karen Xu <[email protected]>

---------

Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen X <[email protected]>
kh3ra pushed a commit to kh3ra/OpenSearch that referenced this pull request Sep 5, 2025
… parity with HTTP (opensearch-project#18925)

* [GRPC] Add proper GRPC status code and error handling

Signed-off-by: Karen Xu <[email protected]>

* rename Http to Rest, group together some convertRestToGrpcStatus codes

Signed-off-by: Karen Xu <[email protected]>

* make final

Signed-off-by: Karen Xu <[email protected]>

---------

Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen X <[email protected]>
vinaykpud pushed a commit to vinaykpud/OpenSearch that referenced this pull request Sep 26, 2025
… parity with HTTP (opensearch-project#18925)

* [GRPC] Add proper GRPC status code and error handling

Signed-off-by: Karen Xu <[email protected]>

* rename Http to Rest, group together some convertRestToGrpcStatus codes

Signed-off-by: Karen Xu <[email protected]>

* make final

Signed-off-by: Karen Xu <[email protected]>

---------

Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen X <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 3.2 Backport to 3.2 branch enhancement Enhancement or improvement to existing feature or request Libraries Lucene Upgrades and Libraries, Any 3rd party library that Core depends on, ex: nebula; team is respo

Projects

Status: Done/Won't Do

Development

Successfully merging this pull request may close these issues.

[Feature Request] GRPC Status Codes and Error Handling

4 participants