Merge pull request #1 from going-dev/feat/surface-google-errorresponse

Surface Google's HTTP-200 ErrorResponse envelope as a typed error

Author: alexwitherspoon

fli/search/__init__.py

@@ -1,5 +1,6 @@
 from .dates import DatePrice, SearchDates
 from .exceptions import (
+    GoogleFlightsUpstreamError,
     SearchClientError,
     SearchConnectionError,
     SearchHTTPError,
@@ -15,4 +16,5 @@
     "SearchTimeoutError",
     "SearchConnectionError",
     "SearchHTTPError",
+    "GoogleFlightsUpstreamError",
 ]

fli/search/_wire.py

@@ -29,6 +29,8 @@
 from collections.abc import Iterator
 from typing import Any
 
+from fli.search.exceptions import GoogleFlightsUpstreamError
+
 logger = logging.getLogger(__name__)
 
 _PREFIX = b")]}'"
@@ -94,14 +96,62 @@ def iter_wrb_chunks(body: str | bytes) -> Iterator[Any]:
         yield from _chunks_from_outer(outer)
 
 
+def _grpc_error_in_frame(frame: Any) -> tuple[int, str | None] | None:
+    """Detect Google's structured ErrorResponse envelope in a ``wrb.fr`` frame.
+
+    A success frame is ``["wrb.fr", null, "<json>"]`` with the payload at index
+    2. When Google rejects the request it still answers HTTP 200 but replaces
+    that payload with an error block, e.g.::
+
+        ["wrb.fr", null, null, null, null,
+         [13, null, [["type.googleapis.com/travel.frontend.flights.ErrorResponse", ...]]]]
+
+    The leading int (``13`` here) is a gRPC status code (13 = INTERNAL). Returns
+    ``(grpc_code, type_url)`` when such a block is present, else ``None``.
+    ``bool`` (an ``int`` subclass) and code ``0`` (OK) are deliberately not
+    treated as errors so neither trips a false positive.
+    """
+    if not isinstance(frame, list):
+        return None
+    for element in frame:
+        if not (isinstance(element, list) and len(element) >= 3):
+            continue
+        code = element[0]
+        if type(code) is not int or code == 0:
+            continue
+        try:
+            type_url = element[2][0][0]
+        except (IndexError, TypeError):
+            type_url = None
+        if isinstance(type_url, str) and type_url.endswith(".ErrorResponse"):
+            return code, type_url
+    return None
+
+
 def _chunks_from_outer(outer: Any) -> Iterator[Any]:
-    """Walk a top-level chunk list and yield decoded inner-JSON payloads."""
+    """Walk a top-level chunk list and yield decoded inner-JSON payloads.
+
+    Raises :class:`~fli.search.exceptions.GoogleFlightsUpstreamError` when a
+    frame carries Google's structured ErrorResponse envelope (an HTTP-200
+    upstream rejection). Without this, the error frame's payload slot is null,
+    so the reader would skip it and the caller would see a silent empty result
+    indistinguishable from "no flights".
+    """
     if not isinstance(outer, list):
         return
     for row in outer:
-        if not isinstance(row, list) or len(row) < 3:
+        if not isinstance(row, list):
             continue
-        if row[0] != "wrb.fr":
+        error = _grpc_error_in_frame(row)
+        if error is not None:
+            grpc_code, type_url = error
+            logger.warning(
+                "Google Flights returned an ErrorResponse (gRPC %s, %s)",
+                grpc_code,
+                type_url,
+            )
+            raise GoogleFlightsUpstreamError(grpc_code, type_url=type_url)
+        if len(row) < 3 or row[0] != "wrb.fr":
             continue
         inner = row[2]
         if not isinstance(inner, str) or not inner:

fli/search/exceptions.py

@@ -28,3 +28,60 @@ def __init__(self, message: str, *, status_code: int | None = None):
         """Store the HTTP status alongside the message for richer logging."""
         super().__init__(message)
         self.status_code = status_code
+
+
+# Canonical gRPC status codes (0-16) carried by Google's ErrorResponse envelope.
+# Used only to label the exception message; the numeric code is authoritative.
+_GRPC_CODE_NAMES = {
+    0: "OK",
+    1: "CANCELLED",
+    2: "UNKNOWN",
+    3: "INVALID_ARGUMENT",
+    4: "DEADLINE_EXCEEDED",
+    5: "NOT_FOUND",
+    6: "ALREADY_EXISTS",
+    7: "PERMISSION_DENIED",
+    8: "RESOURCE_EXHAUSTED",
+    9: "FAILED_PRECONDITION",
+    10: "ABORTED",
+    11: "OUT_OF_RANGE",
+    12: "UNIMPLEMENTED",
+    13: "INTERNAL",
+    14: "UNAVAILABLE",
+    15: "DATA_LOSS",
+    16: "UNAUTHENTICATED",
+}
+
+
+class GoogleFlightsUpstreamError(SearchClientError):
+    """Google answered HTTP 200 with a structured ErrorResponse envelope.
+
+    Instead of flight data, the response carries a gRPC status code (e.g.
+    13 = INTERNAL). This is an upstream rejection, not a parse failure and not
+    a genuine "no results" (which still decodes to ``None``/empty). The numeric
+    code is surfaced via ``grpc_code`` so a caller can apply its own retry or
+    alerting policy rather than fli hard-coding one: a default retry during a
+    broad outage would turn a fleet of clients into a retry storm and deepen
+    the upstream's gating of everyone.
+
+    Attributes:
+        grpc_code: The gRPC status code from the envelope (e.g. 13 = INTERNAL),
+            or ``None`` if it could not be extracted.
+        type_url: The protobuf type URL Google attached, when present.
+
+    """
+
+    def __init__(self, grpc_code: int | None, type_url: str | None = None):
+        """Build the error from the gRPC status code and optional type URL."""
+        self.grpc_code = grpc_code
+        self.type_url = type_url
+        name = _GRPC_CODE_NAMES.get(grpc_code) if grpc_code is not None else None
+        if grpc_code is None:
+            label = "unknown"
+        else:
+            label = f"{grpc_code} {name}" if name else str(grpc_code)
+        super().__init__(
+            f"Google Flights rejected the request with an ErrorResponse "
+            f"envelope (gRPC status {label}) instead of flight data. This is "
+            f"an upstream error, not an empty result."
+        )

tests/search/test_wire.py

@@ -2,7 +2,10 @@
 
 import json
 
+import pytest
+
 from fli.search._wire import iter_wrb_chunks, parse_first_wrb_payload
+from fli.search.exceptions import GoogleFlightsUpstreamError
 
 
 def _single_chunk(payload):
@@ -141,3 +144,43 @@ def test_skips_invalid_inner_to_find_second_valid_chunk(self):
         outer = [["wrb.fr", None, bad_inner], ["wrb.fr", None, good_inner]]
         body = ")]}'\n\n" + json.dumps(outer)
         assert parse_first_wrb_payload(body) == [42]
+
+
+_ERROR_TYPE_URL = "type.googleapis.com/travel.frontend.flights.ErrorResponse"
+
+
+def _error_envelope(grpc_code=13, type_url=_ERROR_TYPE_URL):
+    """Build Google's HTTP-200 ErrorResponse envelope.
+
+    The success payload slot (index 2) is null; the error block carries the
+    gRPC status code and the ErrorResponse type URL.
+    """
+    outer = [["wrb.fr", None, None, None, None, [grpc_code, None, [[type_url]]]]]
+    return ")]}'\n\n" + json.dumps(outer)
+
+
+class TestErrorEnvelope:
+    def test_iter_raises_with_grpc_code(self):
+        with pytest.raises(GoogleFlightsUpstreamError) as exc:
+            list(iter_wrb_chunks(_error_envelope(13)))
+        assert exc.value.grpc_code == 13
+        assert exc.value.type_url.endswith(".ErrorResponse")
+        assert "13 INTERNAL" in str(exc.value)
+
+    def test_parse_first_raises(self):
+        with pytest.raises(GoogleFlightsUpstreamError):
+            parse_first_wrb_payload(_error_envelope(8))
+
+    def test_success_frame_still_parses(self):
+        # Detection must not disturb a normal response.
+        body = _single_chunk([1, "ok", [2, 3]])
+        assert list(iter_wrb_chunks(body)) == [[1, "ok", [2, 3]]]
+
+    def test_non_error_type_url_does_not_false_positive(self):
+        # An int-leading block whose type URL is not an ErrorResponse is ignored.
+        body = _error_envelope(13, type_url="type.googleapis.com/foo.Bar")
+        assert list(iter_wrb_chunks(body)) == []
+
+    def test_grpc_code_zero_is_not_an_error(self):
+        # Code 0 is OK; it must not raise.
+        assert list(iter_wrb_chunks(_error_envelope(0))) == []