fix: skip local quota spend on DNS/transport failures

[djbclark]

Summary

• Local YouTube quota ledger no longer charges for failures with no HTTP response (DNS outages, connection errors, pre-header timeouts).
• Usage events are still recorded as transport_error with units=0 for forensics.
• HTTP 4xx/5xx from Google remain charged (Google counts those).

Reproduces/fixes the 2026-08-01 incident where ~200 OAuth DNS failures burned the local day budget.

Test plan

• python3 -m unittest tests.test_youtube_quota
• Deploy to ~/.superbrain-server and restart playlist sync when idle

Made with Cursor

Summary by CodeRabbit

• Bug Fixes
  • Network failures such as DNS, connection, TLS, and timeout errors are now correctly identified as transport errors.
  • Failed requests without an HTTP response no longer consume API quota.
  • HTTP error responses continue to be charged according to published quota costs.
  • Failure records now accurately distinguish network outages from HTTP service errors.

[coderabbitai]

📝 Walkthrough

Walkthrough

The quota module now detects wrapped DNS, connection, TLS, and pre-response timeout failures. It records these failures with zero quota units and excludes them from ledger updates. Tests cover classification, HTTP 503 distinction, and repeated DNS failures.

Changes

YouTube transport failure accounting

Layer / File(s)	Summary
Transport failure classification backend/core/youtube_quota.py, backend/tests/test_youtube_quota.py	The module traverses wrapped exceptions and classifies eligible pre-response failures as transport_error. Tests cover DNS failures and distinguish them from HTTP 503 responses.
Uncharged usage recording and ledger handling backend/core/youtube_quota.py, backend/tests/test_youtube_quota.py	Transport failures record zero units and skip quota ledger updates. HTTP failures remain charged when quota costs are known. End-to-end tests verify repeated DNS failures consume no quota units.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant YouTubeRequest
  participant classify_result
  participant UsageRecording
  participant QuotaLedger
  YouTubeRequest->>classify_result: raised DNS or other transport exception
  classify_result->>classify_result: traverse exception chain
  classify_result-->>UsageRecording: transport_error without HTTP status
  UsageRecording->>UsageRecording: record zero quota units
  UsageRecording-->>QuotaLedger: skip ledger update

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: avoiding local quota charges for DNS and transport failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/quota-skip-transport-ledger-charges

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

backend/core/youtube_quota.py (1)

154-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the dead requests.exceptions.* extension; a bare OSError check already covers it.

requests.exceptions.RequestException subclasses IOError (an alias of OSError in Python 3), and ConnectionError, Timeout, ConnectTimeout, ReadTimeout, and SSLError all derive from RequestException. Built-in ConnectionError and TimeoutError are also OSError subclasses. So transport_types = (ConnectionError, TimeoutError, OSError) already reduces to OSError, and the local import requests plus the tuple extension at lines 162-168 add no new matches — isinstance(exc, OSError) alone already catches every listed requests.exceptions.* type.

This dead block is also the source of all three static-analysis findings (RUF005 concatenation, S110 try-except-pass, BLE001 blind except). Removing it fixes all three at once. Keep the name-based fallback at lines 174-183: urllib3-native classes like MaxRetryError and NameResolutionError derive from urllib3's HTTPError(Exception), not OSError, so that part is still load-bearing.

♻️ Proposed simplification

-    transport_types = (
-        ConnectionError,
-        TimeoutError,
-        OSError,
-    )
-    try:
-        import requests
-
-        transport_types = transport_types + (
-            requests.exceptions.ConnectionError,
-            requests.exceptions.Timeout,
-            requests.exceptions.ConnectTimeout,
-            requests.exceptions.ReadTimeout,
-            requests.exceptions.SSLError,
-        )
-    except Exception:
-        pass
+    # ConnectionError/TimeoutError are OSError subclasses, and every
+    # requests.exceptions.* transport error subclasses OSError via
+    # RequestException(IOError), so a single OSError check covers them all.
+    transport_types = (OSError,)

Since this depends on the exception hierarchy of requests and CPython's OSError/IOError aliasing, please confirm this holds for the requests version pinned in this repository before merging the simplification.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/core/youtube_quota.py` around lines 154 - 170, Remove the local
requests import, broad exception handler, and transport_types tuple extension in
the transport exception setup. Keep transport_types as the built-in (effectively
OSError) types and preserve the existing name-based fallback for urllib3-native
exceptions such as MaxRetryError and NameResolutionError. Confirm the pinned
requests version retains the stated RequestException hierarchy before applying
the simplification.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@backend/core/youtube_quota.py`:
- Around line 154-170: Remove the local requests import, broad exception
handler, and transport_types tuple extension in the transport exception setup.
Keep transport_types as the built-in (effectively OSError) types and preserve
the existing name-based fallback for urllib3-native exceptions such as
MaxRetryError and NameResolutionError. Confirm the pinned requests version
retains the stated RequestException hierarchy before applying the
simplification.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 254a0b59-cee8-4b63-9d02-752d6120024e

📥 Commits

Reviewing files that changed from the base of the PR and between 30d5613 and f8e764b.

📒 Files selected for processing (2)

• backend/core/youtube_quota.py
• backend/tests/test_youtube_quota.py