[client] Use application specific error code to propagate ray errors #18278
Conversation
There was a problem hiding this comment.
@ckw017 Can you change this reference to use a different error code?
| # Note: https://grpc.github.io/grpc/core/md_doc_statuscodes.html | ||
| # INVALID_ARGUMENT used here since it should never be generated by the | ||
| # grpc lib -- this way we know the error was generated by ray logic | ||
| context.set_code(grpc.StatusCode.INVALID_ARGUMENT) |
There was a problem hiding this comment.
Would ABORTED be a more appropriate error code here? Conventions in my past team is to use ABORTED to signal application errors that are potentially retriable.
python/ray/util/client/worker.py
Outdated
| def decode_exception(data) -> Exception: | ||
| data = base64.standard_b64decode(data) | ||
| def decode_exception(e: grpc.RpcError) -> Exception: | ||
| if e.code() != grpc.StatusCode.INVALID_ARGUMENT: |
There was a problem hiding this comment.
Is it possible to match the full list from the reference you provided? Then using another GRPC error code from the list would still be correct.
There was a problem hiding this comment.
For this case, we should explicitly raise anything that isn't specifically the used in return_exception_in_context, otherwise we'll attempt to deserialize details that probably aren't a serialized ray exception, which would cause a deserialization error that obfuscates the real error.
Why are these changes needed?
At the moment there's no way to distinguish error's from ray from grpc INTERNAL errors. This works around this by using an error code that is never generated by the grpc library (details at the bottom of https://grpc.github.io/grpc/core/md_doc_statuscodes.html)
Fixes:
binascii.Error: Invalid base64-encoded string: number of data characters (21) cannot be 1 more than a multiple of 4and_pickle.UnpicklingError: invalid load key, '%'.when decoding a grpc lib INTERNAL errorRelated issue number
Checks
scripts/format.shto lint the changes in this PR.