[serve] Return "Internal Server Error" instead of traceback on user e…

…xception (ray-project#48491)

In the non-fastapi HTTP handling codepath, we currently format the
internal stack trace and return it in the response. This is unexpected
behavior compared to other HTTP servers because it leaks internal
implementation details.

This change matches the FastAPI behavior: returning a `500` status with
"Internal Server Error."

---------

Signed-off-by: Edward Oakes <[email protected]>

python/ray/serve/_private/replica.py

@@ -59,7 +59,7 @@
 from ray.serve._private.metrics_utils import InMemoryMetricsStore, MetricsPusher
 from ray.serve._private.thirdparty.get_asgi_route_name import get_asgi_route_name
 from ray.serve._private.utils import get_component_file_name  # noqa: F401
-from ray.serve._private.utils import parse_import_path, wrap_to_ray_error
+from ray.serve._private.utils import parse_import_path
 from ray.serve._private.version import DeploymentVersion
 from ray.serve.config import AutoscalingConfig
 from ray.serve.deployment import Deployment
@@ -414,7 +414,7 @@ def _wrap_user_method_call(
                 task.cancel()
         except Exception as e:
             user_exception = e
-            logger.error(f"Request failed:\n{e}")
+            logger.exception("Request failed.")
             if ray.util.pdb._is_ray_debugger_enabled():
                 ray.util.pdb._post_mortem()
         finally:
@@ -1234,15 +1234,16 @@ async def call_user_method(
                 asgi_args=asgi_args,
             )
 
-        except Exception as e:
-            e = wrap_to_ray_error(user_method_name, e)
+        except Exception:
             if request_metadata.is_http_request and asgi_args is not None:
-                result = starlette.responses.Response(
-                    f"Unexpected error, traceback: {e}.", status_code=500
+                await self._send_user_result_over_asgi(
+                    starlette.responses.Response(
+                        "Internal Server Error", status_code=500
+                    ),
+                    asgi_args,
                 )
-                await self._send_user_result_over_asgi(result, asgi_args)
 
-            raise e from None
+            raise
         finally:
             if receive_task is not None and not receive_task.done():
                 receive_task.cancel()

python/ray/serve/_private/utils.py

@@ -7,7 +7,6 @@
 import random
 import string
 import time
-import traceback
 import uuid
 from abc import ABC, abstractmethod
 from decimal import ROUND_HALF_UP, Decimal
@@ -24,7 +23,6 @@
 from ray._private.worker import LOCAL_MODE, SCRIPT_MODE
 from ray._raylet import MessagePackSerializer
 from ray.actor import ActorHandle
-from ray.exceptions import RayTaskError
 from ray.serve._private.common import ServeComponentType
 from ray.serve._private.constants import HTTP_PROXY_TIMEOUT, SERVE_LOGGER_NAME
 from ray.types import ObjectRef
@@ -160,17 +158,6 @@ def ensure_serialization_context():
     ray.util.serialization_addons.apply(ctx)
 
 
-def wrap_to_ray_error(function_name: str, exception: Exception) -> RayTaskError:
-    """Utility method to wrap exceptions in user code."""
-
-    try:
-        # Raise and catch so we can access traceback.format_exc()
-        raise exception
-    except Exception as e:
-        traceback_str = ray._private.utils.format_error_message(traceback.format_exc())
-        return ray.exceptions.RayTaskError(function_name, traceback_str, e)
-
-
 def msgpack_serialize(obj):
     ctx = ray._private.worker.global_worker.get_serialization_context()
     buffer = ctx.serialize(obj)

python/ray/serve/api.py

@@ -193,7 +193,7 @@ def decorator(cls):
 
         if issubclass(cls, collections.abc.Callable):
             raise ValueError(
-                "Class passed to @serve.ingress may not have __call__ method."
+                "Classes passed to @serve.ingress may not have __call__ method."
             )
 
         # Sometimes there are decorators on the methods. We want to fix

python/ray/serve/tests/test_http_routes.py

@@ -220,7 +220,7 @@ def f():
     serve.run(f.bind())
     r = requests.get("http://localhost:8000/f")
     assert r.status_code == 500
-    assert "ZeroDivisionError" in r.text, r.text
+    assert r.text == "Internal Server Error"
 
     @ray.remote(num_cpus=0)
     def intentional_kill(actor_handle):

python/ray/serve/tests/test_logging.py

@@ -738,20 +738,21 @@ def disable_stdout():
             with open(logs_dir / log_file) as f:
                 for line in f:
                     structured_log = json.loads(line)
-                    _message = structured_log["message"]
-                    if "from_serve_logger" in _message:
+                    message = structured_log["message"]
+                    exc_text = structured_log.get("exc_text", "")
+                    if "from_serve_logger" in message:
                         from_serve_logger_check = True
-                    elif "from_print" in _message:
+                    elif "from_print" in message:
                         from_print_check = True
 
                     # Error was logged from replica directly.
-                    elif "from_error" in _message:
+                    elif "from_error" in exc_text:
                         from_error_check = True
-                    elif "direct_from_stdout" in _message:
+                    elif "direct_from_stdout" in message:
                         direct_from_stdout = True
-                    elif "direct_from_stderr" in _message:
+                    elif "direct_from_stderr" in message:
                         direct_from_stderr = True
-                    elif "this\nis\nmultiline\nlog\n" in _message:
+                    elif "this\nis\nmultiline\nlog\n" in message:
                         multiline_log = True
     assert from_serve_logger_check
     assert from_print_check

python/ray/serve/tests/unit/test_user_callable_wrapper.py

@@ -12,7 +12,6 @@
 from starlette.responses import PlainTextResponse
 
 from ray import serve
-from ray.exceptions import RayTaskError
 from ray.serve._private.common import (
     DeploymentID,
     RequestMetadata,
@@ -152,7 +151,7 @@ def test_basic_class_callable():
 
     # Call non-generator method with is_streaming.
     request_metadata = _make_request_metadata(is_streaming=True)
-    with pytest.raises(RayTaskError, match="did not return a generator."):
+    with pytest.raises(TypeError, match="did not return a generator."):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), dict()
         ).result()
@@ -176,7 +175,7 @@ def test_basic_class_callable():
         ).result()
         == "hi-kwarg"
     )
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), {"raise_exception": True}
         ).result()
@@ -185,7 +184,7 @@ def test_basic_class_callable():
     request_metadata = _make_request_metadata(
         call_method="call_async", is_streaming=True
     )
-    with pytest.raises(RayTaskError, match="did not return a generator."):
+    with pytest.raises(TypeError, match="did not return a generator."):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), dict()
         ).result()
@@ -210,7 +209,7 @@ def test_basic_class_callable():
         ).result()
         == "hi-kwarg"
     )
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), {"raise_exception": True}
         ).result()
@@ -230,7 +229,7 @@ async def append_to_list(item: Any):
         call_method="call_generator", is_streaming=False
     )
     with pytest.raises(
-        RayTaskError, match="Method 'call_generator' returned a generator."
+        TypeError, match="Method 'call_generator' returned a generator."
     ):
         user_callable_wrapper.call_user_method(
             request_metadata, (10,), dict(), generator_result_callback=append_to_list
@@ -247,7 +246,7 @@ async def append_to_list(item: Any):
     result_list.clear()
 
     # Call sync generator raising exception.
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata,
             (10,),
@@ -262,7 +261,7 @@ async def append_to_list(item: Any):
         call_method="call_async_generator", is_streaming=False
     )
     with pytest.raises(
-        RayTaskError, match="Method 'call_async_generator' returned a generator."
+        TypeError, match="Method 'call_async_generator' returned a generator."
     ):
         user_callable_wrapper.call_user_method(
             request_metadata, (10,), dict(), generator_result_callback=append_to_list
@@ -279,7 +278,7 @@ async def append_to_list(item: Any):
     result_list.clear()
 
     # Call async generator raising exception.
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata,
             (10,),
@@ -296,7 +295,7 @@ def test_basic_function_callable(fn: Callable):
 
     # Call non-generator function with is_streaming.
     request_metadata = _make_request_metadata(is_streaming=True)
-    with pytest.raises(RayTaskError, match="did not return a generator."):
+    with pytest.raises(TypeError, match="did not return a generator."):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), dict()
         ).result()
@@ -317,7 +316,7 @@ def test_basic_function_callable(fn: Callable):
             request_metadata, tuple(), {"suffix": "-kwarg"}
         ).result()
     ) == "hi-kwarg"
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata, tuple(), {"raise_exception": True}
         ).result()
@@ -336,7 +335,7 @@ async def append_to_list(item: Any):
     # Call generator function without is_streaming.
     request_metadata = _make_request_metadata(is_streaming=False)
     with pytest.raises(
-        RayTaskError, match=f"Method '{fn.__name__}' returned a generator."
+        TypeError, match=f"Method '{fn.__name__}' returned a generator."
     ):
         user_callable_wrapper.call_user_method(
             request_metadata, (10,), dict(), generator_result_callback=append_to_list
@@ -353,7 +352,7 @@ async def append_to_list(item: Any):
     result_list.clear()
 
     # Call generator function raising exception.
-    with pytest.raises(RayTaskError, match="uh-oh"):
+    with pytest.raises(RuntimeError, match="uh-oh"):
         user_callable_wrapper.call_user_method(
             request_metadata,
             (10,),