fix(search): detect and retry Google's HTTP-200 ErrorResponse envelope (#200)

[alecsalisbury]

Summary

Fixes #200 — searches intermittently return zero results (and the Python API returns None) even on trunk routes, with no error.

Root cause

GetShoppingResults / GetCalendarGraph sometimes answer a perfectly valid request with HTTP 200 whose wrb.fr row carries no inner data payload, but instead a typed gRPC error:

[["wrb.fr",null,null,null,null,[13,null,
  [["type.googleapis.com/travel.frontend.flights.ErrorResponse", ...]]]]]

The 13 is a canonical gRPC status (INTERNAL). Because the HTTP status is 200, the client's retry never fires, and the wire parser silently drops the unrecognised row (row[2] isn't a string) — so parse_first_wrb_payload returns None, search() returns None, and the CLI/MCP report "No flights found." During a transient Google-side spell this makes every query look like a permanent empty result, which is exactly what #200 reporters saw across versions 0.6.0–0.10.0.

I reproduced it directly: the same request alternates between a full ~218 KB result and a 360-byte ErrorResponse envelope from one call to the next.

Fix

• _wire.wrb_error_code() — detects the error envelope and returns its gRPC code (chunk framing refactored into a shared _iter_outer generator; iter_wrb_chunks now delegates to it).
• SearchBackendError — typed exception carrying the code. Transient codes (INTERNAL, UNAVAILABLE, …) are retryable; request-error codes (INVALID_ARGUMENT, NOT_FOUND, UNAUTHENTICATED, …) fail fast so we don't hammer Google with a request it will always reject.
• client.post_rpc() — a POST helper that raises SearchBackendError on the envelope and retries transient codes with exponential backoff. The shopping, calendar, and booking calls now route through it.

Genuine zero-result responses (a normal success chunk with empty arrays) are unaffected and still return None.

Tests

• tests/search/test_wire.py — envelope detection (codes, sentinel for non-int, ignores non-error type URLs, doesn't leak as a phantom chunk).
• tests/search/test_client.py — post_rpc returns body on success, retries transient codes then succeeds, exhausts retries and raises, and fails fast on non-retryable codes.

ruff check clean. New tests are self-contained (no live API). The existing tests/search/ suite hits the live API and is out of scope here.

🤖 Generated with Claude Code

Greptile Summary

This PR adds detection and retry logic for Google's HTTP-200 ErrorResponse envelope — a gRPC-level error disguised behind a 200 status that previously decoded silently as "no results found" (issue #200). The fix is layered cleanly: _iter_outer separates raw chunk framing from payload parsing, wrb_error_code inspects the outer row shape to extract the gRPC status code, SearchBackendError classifies codes as retryable or not, and post_rpc wraps every RPC call with tenacity retry for transient codes.

• _wire.py is refactored into _iter_outer (raw framing) + iter_wrb_chunks (inner payloads); the refactor is behavior-preserving and the new wrb_error_code / _is_error_block helpers correctly detect the error envelope shape from real Google responses.
• client.py introduces post_rpc with up to 4 exponential-backoff retries for transient gRPC codes, while permanent request-error codes (INVALID_ARGUMENT, NOT_FOUND, etc.) fail fast to avoid hammering Google.
• Tests provide solid unit coverage of envelope detection, sentinel handling, retry exhaustion, and fast-fail behavior — all without live API calls.

Confidence Score: 4/5

Safe to merge — the fix is targeted, the refactor is behavior-preserving, and the new retry path is well-tested with offline unit tests.

The core logic is sound and the test coverage is thorough. The one structural note is a redundant raise_for_status() call in post_rpc that Client.post already handles internally — harmless today but could mislead a future reader into thinking the check is meaningful there.

No files require special attention; fli/search/client.py has the minor redundancy noted above.

Important Files Changed

Filename	Overview
fli/search/_wire.py	Refactored into _iter_outer (raw framing) + iter_wrb_chunks (inner payload); added wrb_error_code and _is_error_block for HTTP-200 error envelope detection. Logic is correct and behaviorally equivalent to the original.
fli/search/client.py	Adds post_rpc with tenacity retry for SearchBackendError; contains one redundant response.raise_for_status() call since Client.post already validates HTTP status internally.
fli/search/exceptions.py	Adds SearchBackendError with gRPC-code-based retryable property; NON_RETRYABLE frozenset correctly targets permanent request-error codes and leaves transient codes retryable.
fli/search/flights.py	Swaps direct client.post calls for post_rpc in shopping, calendar, and booking flows; mechanical refactor with no logic change beyond error detection.
fli/search/dates.py	Same mechanical swap as flights.py — direct client.post replaced with post_rpc; straightforward and correct.
tests/search/test_client.py	Good new tests for post_rpc retry logic; _no_backoff_sleep fixture is autouse=True at module scope, unnecessarily patching retry state for every test in the file including unrelated ones.
tests/search/test_wire.py	Comprehensive new TestWrbErrorCode class covers detection of error envelopes, sentinel for non-int codes, type-URL filtering, bytes input, and confirms error envelopes don't leak as phantom data chunks.

Sequence Diagram

sequenceDiagram
    participant Caller as SearchFlights / SearchDates
    participant PR as post_rpc (tenacity, ≤4 attempts)
    participant CP as Client.post (tenacity, ≤3 attempts)
    participant G as Google Flights API

    Caller->>PR: post_rpc(client, url, encoded)
    PR->>CP: "client.post(url, f.req=…)"
    CP->>G: "POST f.req=…"
    G-->>CP: HTTP 200 (ErrorResponse envelope)
    CP-->>PR: Response (raise_for_status → no-op, 200)
    PR->>PR: wrb_error_code(response.text) → 13 (INTERNAL)
    PR->>PR: "raise SearchBackendError(code=13, retryable=True)"
    Note over PR: tenacity: wait exponential, retry
    PR->>CP: "client.post(url, f.req=…)"
    CP->>G: "POST f.req=…"
    G-->>CP: HTTP 200 (full data payload)
    CP-->>PR: Response
    PR->>PR: wrb_error_code → None (no error)
    PR-->>Caller: response.text (full body)

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
fli/search/client.py:254-256
`response.raise_for_status()` here is dead code. `Client.post` already calls `raise_for_status()` inside its own `try` block — any non-2xx response is caught, wrapped into `SearchHTTPError`, and re-raised before the `Response` object is ever returned. The call here will always be a no-op for a 200.

```suggestion
    code = wrb_error_code(response.text)
```

### Issue 2 of 2
tests/search/test_client.py:125-131
`autouse=True` at module scope applies this fixture to every test in the file — including `TestHostFromUrl`, `TestWrapRequestError`, and `TestGetClientSingleton` — even though none of them exercise `post_rpc`. The fixture mutates and then restores `post_rpc.retry.wait` on every test run unnecessarily; limiting the scope to the class that actually needs it avoids the spurious global-state churn.

```suggestion
@pytest.fixture()
def _no_backoff_sleep():
    """Strip the exponential wait from post_rpc so retry tests run instantly."""
    original = post_rpc.retry.wait
    post_rpc.retry.wait = wait_none()
    yield
    post_rpc.retry.wait = original
```

_{Reviews (1): Last reviewed commit: "fix(search): detect and retry Google's H..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[greptile-apps]

response.raise_for_status() here is dead code. Client.post already calls raise_for_status() inside its own try block — any non-2xx response is caught, wrapped into SearchHTTPError, and re-raised before the Response object is ever returned. The call here will always be a no-op for a 200.

Suggested change

	response.raise_for_status()
	code = wrb_error_code(response.text)
	code = wrb_error_code(response.text)

Prompt To Fix With AI

This is a comment left during a code review.
Path: fli/search/client.py
Line: 254-256

Comment:
`response.raise_for_status()` here is dead code. `Client.post` already calls `raise_for_status()` inside its own `try` block — any non-2xx response is caught, wrapped into `SearchHTTPError`, and re-raised before the `Response` object is ever returned. The call here will always be a no-op for a 200.

```suggestion
    code = wrb_error_code(response.text)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

autouse=True at module scope applies this fixture to every test in the file — including TestHostFromUrl, TestWrapRequestError, and TestGetClientSingleton — even though none of them exercise post_rpc. The fixture mutates and then restores post_rpc.retry.wait on every test run unnecessarily; limiting the scope to the class that actually needs it avoids the spurious global-state churn.

Suggested change

	@pytest.fixture(autouse=True)
	def _no_backoff_sleep():
	"""Strip the exponential wait from post_rpc so retry tests run instantly."""
	original = post_rpc.retry.wait
	post_rpc.retry.wait = wait_none()
	yield
	post_rpc.retry.wait = original
	@pytest.fixture()
	def _no_backoff_sleep():
	"""Strip the exponential wait from post_rpc so retry tests run instantly."""
	original = post_rpc.retry.wait
	post_rpc.retry.wait = wait_none()
	yield
	post_rpc.retry.wait = original

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/search/test_client.py
Line: 125-131

Comment:
`autouse=True` at module scope applies this fixture to every test in the file — including `TestHostFromUrl`, `TestWrapRequestError`, and `TestGetClientSingleton` — even though none of them exercise `post_rpc`. The fixture mutates and then restores `post_rpc.retry.wait` on every test run unnecessarily; limiting the scope to the class that actually needs it avoids the spurious global-state churn.

```suggestion
@pytest.fixture()
def _no_backoff_sleep():
    """Strip the exponential wait from post_rpc so retry tests run instantly."""
    original = post_rpc.retry.wait
    post_rpc.retry.wait = wait_none()
    yield
    post_rpc.retry.wait = original
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[alexechoi]

This is the strongest architecture of the open error-envelope PRs — detection at the RPC boundary where the tenacity retry already lives, and a typed exception that composes with the CLI's error handling. Two pieces of first-hand evidence from reverse-engineering GetExploreDestinations (#226) that I think should shape the final version:

1. Code 13 is not reliably transient — the retry classification is inverted for a whole class of failures.
While probing the explore endpoint I produced this exact envelope deterministically from permanently-malformed requests — a missing departure date, a wrong location type code, and a missing same-origin header each return code 13, 100% reproducibly:

["wrb.fr",null,null,null,null,[13,null,[["type.googleapis.com/travel.frontend.flights.ErrorResponse",[[null,null,0,"<request-id>"],0]]]]]

Under NON_RETRYABLE = {3, 5, 7, 9, 11, 16}, those permanently-bad requests get classified as retryable and burn 4 attempts with backoff each. Combined with the existing fan-outs (parallel_map in dates chunking, round-trip N+1 expansion), one malformed request becomes a small request storm against the 10 req/s bucket. Notably, the #200 thread contains three mutually exclusive diagnoses of this same envelope (transient INTERNAL, per-fingerprint throttle, IPv6 reputation) — and under two of them, retrying makes things worse. Suggested policy: surface by default, retry 13 at most once with real backoff, and never auto-retry inside a fan-out.

2. The detector misses the bare variant.
Google also emits ["wrb.fr",null,null,null,null,[13]] with no details block (#224 has a capture). wrb_error_code's _is_error_block requires a string containing "Error" in block[2], so that variant silently falls back to "no flights found". #224's structural check (row[5][0] is an int) catches both shapes — worth adopting as the gate, keeping the type URL as enrichment rather than as the detector.

3. Consolidation map. Four open PRs now address this same bug (#201, #205, #208, #224), and they will collide — e.g. this PR and #224 both add a SearchBackendError with incompatible kwargs (code= vs error_code=). FWIW the best merged shape looks like: this PR's post_rpc boundary + typed exception as the base; #224's structural detector; #208's MCP error payloads, dates.py coverage and captured fixture (with its impersonate-rotation commit split out); #205 closed as subsumed. One maintainer decision on the exception name/kwarg would unblock all four.