Merge pull request #1013 from lupko/flight-fixes

RELATED: CQ-1268 - add limits to ErrorInfo fields

Author: lupko

gooddata-flight-server/gooddata_flight_server/errors/error_info.py

@@ -1,13 +1,29 @@
 #  (C) 2024 GoodData Corporation
 import base64
 import traceback
-from typing import Callable, Optional, Union
+from typing import Callable, Optional, Union, cast
 
 import orjson
 import pyarrow.flight
 
 from gooddata_flight_server.errors.error_code import ErrorCode
 
+_ERROR_INFO_MAX_MSG = 256
+_ERROR_INFO_MAX_DETAIL = 512
+
+
+def _truncate_str_value(val: Optional[str], max_len: int) -> Optional[str]:
+    if val is None:
+        return None
+
+    if len(val) <= max_len:
+        return val
+
+    # no big deal that the actual max length is slightly exceeded
+    # all this truncating happens because ErrorInfo is eventually
+    # passed via gRPC headers which have 16k hard limit
+    return val[:max_len] + " [truncated]"
+
 
 class ErrorInfo:
     """
@@ -22,15 +38,15 @@ def __init__(
         body: Optional[bytes] = None,
         code: int = 0,
     ) -> None:
-        self._msg = msg
-        self._detail: Optional[str] = detail
+        self._msg = cast(str, _truncate_str_value(msg, _ERROR_INFO_MAX_MSG))
+        self._detail: Optional[str] = _truncate_str_value(detail, _ERROR_INFO_MAX_DETAIL)
         self._body: Optional[bytes] = body
         self._code: int = code
 
     @property
     def msg(self) -> str:
         """
-        :return: human readable error message
+        :return: human-readable error message
         """
         return self._msg
 
@@ -60,26 +76,36 @@ def with_msg(self, msg: str) -> "ErrorInfo":
         """
         Updates error message.
 
-        :param msg: new message
+        :param msg: new message, up to 256 characters; will be truncated if the limit is exceeded
         :return: self, for call chaining sakes
         """
-        self._msg = msg
+        self._msg = cast(str, _truncate_str_value(msg, _ERROR_INFO_MAX_MSG))
+
         return self
 
     def with_detail(self, detail: Optional[str] = None) -> "ErrorInfo":
         """
         Updates or resets the error detail.
 
-        :param detail: detail to set; if None, the detail stored in the meta will be removed; default is None
+        :param detail: detail to set; if None, the detail stored in the meta will be removed; default is None;
+         detail can be up to 512 characters; will be truncated if the limit is exceeded
         :return: self, for call chaining sakes
         """
-        self._detail = detail
+        self._detail = _truncate_str_value(detail, _ERROR_INFO_MAX_DETAIL)
+
         return self
 
     def with_body(self, body: Optional[Union[bytes, str]]) -> "ErrorInfo":
         """
         Updates or resets the error body.
 
+        IMPORTANT: the ErrorInfo (and thus the contents of `body`) are passed out via FlightError.extra_info
+        property. The Flight RPC implementations pass the `extra_info` via gRPC headers. In turn, the gRPC headers
+        do have size limit. Keep this in mind when designing the value of `body`.
+
+        If you set body that is too large, you will run into problems like this:
+         https://github.com/grpc/grpc/issues/37852.
+
         :param body: body to set; if None, the body stored in the meta will be removed; default is None
         :return: self, for call chaining sakes
         """

gooddata-flight-server/tests/errors/error_info.py

@@ -22,6 +22,17 @@ def test_serde_error_info2():
     assert deserialized.detail == error.detail
 
 
+def test_error_info_limits1():
+    error = ErrorInfo.for_reason(666, "error" * 500).with_detail("detail" * 500)
+
+    assert "truncated" in error.msg
+    assert "truncated" in error.detail
+
+    error = ErrorInfo(code=666, msg="error" * 500, detail="detail" * 500)
+    assert "truncated" in error.msg
+    assert "truncated" in error.detail
+
+
 def test_serde_retry_info1():
     retry = RetryInfo(
         flight_info=None,