Skip to content

Comments

fix: API key on /search + client-side rate limiting (#155, #154)#156

Merged
deucebucket merged 5 commits intodevelopfrom
fix/issue-155-api-key-and-rate-limiting
Feb 16, 2026
Merged

fix: API key on /search + client-side rate limiting (#155, #154)#156
deucebucket merged 5 commits intodevelopfrom
fix/issue-155-api-key-and-rate-limiting

Conversation

@deucebucket
Copy link
Owner

Summary

  • fix: API key not sent on /search requests #155: Add X-API-Key header to all Skaldleita /search requests (was only sent on /match). Updates both search_bookdb_api() pipeline function and the manual search route. Fixes 403 Forbidden errors since Skaldleita now requires auth on all endpoints.
  • feat: Add client-side rate limiting and Retry-After support #154: Add handle_rate_limit_response() utility in rate_limiter.py with exponential backoff (30s→60s→120s), Retry-After header parsing, and circuit breaker integration. Applied to bookdb.py and fingerprint.py. Frontend shows inline rate limit warnings on library, queue, and history pages.

Test plan

  • Verify /search requests now include API key (check Skaldleita server logs)
  • Trigger a 429 response and confirm backoff + retry behavior
  • Confirm rate limit warning appears in UI when 429 received
  • Run test-naming-issues.py (281/281 pass)
  • ruff check --select=F821 clean

@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

PR #156 adds API key authentication to /search requests and implements centralized rate-limiting with Retry-After header support.

Codebase Patterns I Verified

  • Import patterns: Checked existing from library_manager.* imports in app.py (lines 46-109)
  • Error handling: No existing @app.errorhandler decorators in current code
  • Rate limiting: Existing pattern uses inline retry logic with Retry-After header (app.py:3057-3066)
  • API key usage: API key passed via headers dict: {"X-API-Key": api_key} (bookdb.py:144)
  • Module structure: Checked library_manager/ directory - no feedback.py module exists

✅ Good

  • Centralized rate limit logic - handle_rate_limit_response() eliminates duplicated retry code across 3+ locations
  • Consistent auth headers - get_signed_headers() + X-API-Key pattern used across all endpoints
  • Client-side UX - JavaScript handles 429 responses with countdown display and toast notifications
  • Exponential backoff - Falls back to 30 * (2 ** retry_count) when Retry-After header is missing
  • Circuit breaker integration - Rate limit handler respects existing circuit breaker state

🚨 Issues Found

Severity Location Issue Fix
CRITICAL app.py:21-24 MISSING MODULE - Imports from library_manager.feedback which doesn't exist. Code will crash on startup with ModuleNotFoundError. Either create library_manager/feedback.py with required functions OR remove these imports and calls. Check lines 36, 118, 126, 135, 143, 151, 186 where feedback functions are called.
CRITICAL rate_limiter.py:399 MISSING FUNCTION - handle_rate_limit_response() is added to diff (lines 399-458) but the actual file only has 97 lines. Function doesn't exist in current codebase. Verify the function was actually added to library_manager/providers/rate_limiter.py on the PR branch.
HIGH bookdb.py:148-171 INCONSISTENT MIGRATION - The /match endpoint still uses old inline rate limit logic (lines 148-171) while /search uses new handle_rate_limit_response(). One function has two different behaviors. Refactor /match endpoint to use handle_rate_limit_response() for consistency.
HIGH app.py:3073-3077 MISSING API KEY ON FIRST REQUEST - Headers built AFTER rate_limit_wait() and INSIDE the try block. If secrets loading fails, request goes out without auth. Move secrets = load_secrets() and header building BEFORE rate_limit_wait() and OUTSIDE the try block.
MEDIUM app.py:3076-3077 NO SIGNATURE HEADERS - Builds basic headers but doesn't merge with get_signed_headers(). Missing X-LM-Signature and X-LM-Timestamp. Change to: headers = get_signed_headers(); headers['X-API-Key'] = api_key
MEDIUM fingerprint.py:360-362, 372-374, 384-386 UNUSED RETURN VALUES - Calls handle_rate_limit_response() but ignores result, doesn't retry. Function returns should_retry but code just logs and returns None. Either implement retry logic or document why retries are skipped for fingerprint endpoints.
LOW app.py:11019-11020 INCONSISTENT NAMING - Variables named sl_api_key, sl_headers (Skaldleita prefix) but earlier code uses api_key, headers. Use consistent naming: either api_key/headers or sl_api_key/sl_headers throughout.
LOW templates/*.html:481, 502, 522 INCONSISTENT TOAST CHECK - Uses typeof showToast === 'function' check but not everywhere. If showToast doesn't exist, code silently fails. Verify showToast is globally available or add fallback UI feedback.

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key not sent on /search ⚠️ PARTIAL Fixed for /api/search_bookdb (app.py:11019) and search_bookdb_api (app.py:3076), but NOT applied to bookdb.py /match endpoint which still has old code (lines 141-146).
#154 Client-side rate limiting + Retry-After ⚠️ PARTIAL Backend handles Retry-After correctly. Frontend displays rate limit messages. MISSING: No client-side debouncing or request queuing to prevent rapid repeated requests. Issue asks for "debounce rapid requests" but only server-side backoff is implemented.

Scope Status: SCOPE_PARTIAL

Critical Missing Pieces:

  1. library_manager/feedback.py module doesn't exist - code will crash
  2. handle_rate_limit_response() function missing from rate_limiter.py
  3. Client-side debouncing not implemented (Issue feat: Add client-side rate limiting and Retry-After support #154 explicitly requests this)
  4. Inconsistent application - some endpoints migrated, others still use old logic

📝 Documentation Check

CHANGELOG.md:MISSING - This is a fix: PR addressing two bugs (#154, #155). CHANGELOG.md should be updated with entries explaining:

  • Fixed: API key now sent on all Skaldleita endpoints including /search
  • Fixed: Rate limit responses now display retry countdown to users

🎯 Verdict

REQUEST_CHANGES

Blocking Issues:

  1. CRITICAL - Missing library_manager/feedback.py module - code will crash on import
  2. CRITICAL - handle_rate_limit_response() function not in actual file (only in diff)
  3. HIGH - API key headers built INSIDE try block, can fail silently
  4. HIGH - Inconsistent migration - /match endpoint not updated

Required Fixes:

  • Either create library_manager/feedback.py OR remove all feedback imports/calls (8 locations)
  • Verify handle_rate_limit_response() exists in rate_limiter.py on PR branch
  • Move header building outside try block and merge with get_signed_headers()
  • Migrate bookdb.py /match endpoint to use new rate limit handler
  • Update CHANGELOG.md with fix descriptions
  • Consider adding client-side debouncing (Issue feat: Add client-side rate limiting and Retry-After support #154 requirement)

Non-Blocking Recommendations:

  • Document why fingerprint endpoints don't retry on 429
  • Standardize variable naming (sl_api_key vs api_key)
  • Add fallback for missing showToast function

@deucebucket deucebucket force-pushed the fix/issue-155-api-key-and-rate-limiting branch from 2896980 to e00252d Compare February 16, 2026 06:55
@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

PR #156 fixes API key authentication missing on /search endpoints and implements centralized rate-limiting with server-provided Retry-After support.

Codebase Patterns I Verified

  • API key pattern: Existing code uses headers={"X-API-Key": api_key} (bookdb.py:144)
  • Signed headers pattern: get_signed_headers() returns dict with User-Agent, X-LM-Signature, X-LM-Timestamp (bookdb.py:64-82)
  • Rate limiting: Current code has inline retry logic scattered across files (app.py:3057-3066, bookdb.py:148-171)
  • Error handling: Uses logger.info/warning/debug, no custom exception classes
  • Type hints: Not used in this codebase (checked multiple provider files)

✅ Good

  • Fixes root cause: Adds API key header to all /search requests (app.py:3034, 3043; app.py:10951, 10953)
  • Centralized logic: New handle_rate_limit_response() eliminates duplicate retry code across 3 files
  • Exponential backoff: Falls back to 30 * (2 ** retry_count) when Retry-After header missing (rate_limiter.py:309)
  • Circuit breaker integration: Respects existing circuit breaker state (rate_limiter.py:292-295)
  • User feedback: JavaScript displays retry countdown in UI (templates/:365-367)
  • Consistent header building: Uses get_signed_headers() + API key pattern across all endpoints

🚨 Issues Found

Severity Location Issue Fix
HIGH app.py:3031-3034 Headers built in wrong scope - secrets = load_secrets() and header building happen AFTER rate_limit_wait() and INSIDE the try block. If secrets loading fails, the request will still go out without auth. Move lines 3031-3034 (secrets loading and header building) to BEFORE line 3027 (rate_limit_wait), OUTSIDE the try block.
MEDIUM app.py:3034 Missing signed headers - Code builds headers dict manually but doesn't merge with get_signed_headers(). Missing X-LM-Signature and X-LM-Timestamp headers that Skaldleita expects. Change line 3034 to: headers = get_signed_headers() then add headers['X-API-Key'] = api_key on line 3035.
MEDIUM bookdb.py:149-201 Incomplete migration - The /match endpoint (lines 149-201) still uses OLD inline rate-limit logic while this PR migrates /search to use handle_rate_limit_response(). Creates inconsistency. Refactor bookdb.py:149-201 to use handle_rate_limit_response() for consistency. This was probably overlooked because /match already had API keys (issue #155 only mentioned /search).
MEDIUM fingerprint.py:223-225, 235-237, 247-249 Unused return values - Calls handle_rate_limit_response() but ignores should_retry field. Function returns retry logic but code just logs warning and returns None/False. Wastes the backoff calculation. Either: (1) Implement retry loops in fingerprint functions OR (2) Add comment explaining why fingerprint endpoints don't retry (e.g., "fingerprint lookups are optional, fail fast").
LOW app.py:10893-10895 Variable naming inconsistency - Uses sl_api_key and sl_headers (Skaldleita prefix) but earlier in same file uses api_key and headers (lines 3032-3034). Pick one: either prefix all with sl_ or use plain names throughout. Prefer plain names for consistency with existing code.
LOW templates/*.html:365-367 Defensive coding - Uses typeof showToast === 'function' check but showToast doesn't exist in current codebase (grep returned 0 matches). If undefined, check passes but nothing happens - silent failure. Either: (1) Define showToast function in base template OR (2) Add fallback: if (typeof showToast === 'function') showToast(...); else console.warn(...).

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key not sent on /search YES Fixed in search_bookdb_api() (app.py:3031-3034) and /api/search_bookdb (app.py:10893-10895). All GET /search requests now include X-API-Key header.
#154 Client-side rate limiting + Retry-After ⚠️ PARTIAL Server-side Retry-After parsing is complete. UI displays countdown (templates/:365-367). MISSING: Issue #154 explicitly requests "debounce rapid requests" but no client-side request queuing or debouncing implemented. Users can still spam the search button.

Scope Status: SCOPE_PARTIAL

What's Missing from #154:

  • No client-side debouncing (e.g., 500ms delay between keystrokes)
  • No request queue/cancellation (multiple rapid clicks still send multiple requests)
  • Issue description says "debounce rapid requests" - only server-side backoff is implemented

📝 Documentation Check

CHANGELOG.md:MISSING

This is a fix: PR addressing bugs #154 and #155. CHANGELOG.md should be updated with:

## [0.9.0-beta.XXX] - 2026-02-XX

### Fixed

- **Issue #155: API key not sent on /search requests** - All Skaldleita API endpoints now include authentication headers. Previously GET /search requests were missing the X-API-Key header, causing 403 Forbidden errors.
- **Issue #154: Rate limit handling** - Server 429 responses now display retry countdown to users with toast notifications. Backend respects Retry-After header with exponential backoff fallback.

🎯 Verdict

REQUEST_CHANGES

Required Fixes:

  1. HIGH - Move header building outside try block (app.py:3031-3034 before line 3027)
  2. MEDIUM - Merge with get_signed_headers() to include X-LM-Signature (app.py:3034)
  3. CHANGELOG - Add entries for feat: Add client-side rate limiting and Retry-After support #154 and fix: API key not sent on /search requests #155

Recommended Fixes:
4. Migrate bookdb.py /match endpoint to use handle_rate_limit_response() (consistency)
5. Document why fingerprint endpoints don't retry on 429
6. Standardize variable naming (remove sl_ prefix OR use everywhere)
7. Define showToast() function or add console.warn fallback

About Scope:

@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

This PR fixes API key authentication issues on /search requests (#155) and implements centralized rate limiting with exponential backoff (#154).

Codebase Patterns I Verified

  • Logging: Project uses logger = logging.getLogger(__name__) pattern consistently
  • Error handling: Uses except Exception as e with logging, not bare except
  • Type hints: Mixed usage - fingerprint.py has comprehensive type hints (e.g., -> Optional[Dict[str, Any]]), but rate_limiter.py and bookdb.py don't use them consistently
  • Rate limiting: Existing pattern in rate_limiter.py uses record_api_failure() and record_api_success() for circuit breaker management
  • API calls: Existing code shows headers passed via dict, timeouts always specified

✅ Good

  • Centralized rate limit handling - The new handle_rate_limit_response() function eliminates duplicate code across providers
  • Exponential backoff - Implements proper 30s/60s/120s progression with 5-min cap
  • Retry-After parsing - Correctly parses server response and respects the header
  • Circuit breaker integration - Calls record_api_failure() and checks circuit state before retrying
  • Frontend feedback - Users get clear "rate limited" messages with retry countdown
  • API key added consistently - All search endpoints now include X-API-Key header
  • CHANGELOG updated - Both issues properly documented

🚨 Issues Found

Severity Location Exact Code Quote Issue Fix
HIGH bookdb.py:149-171 Lines 149-171: Old rate limit code still exists The diff shows handle_rate_limit_response() should REPLACE the old manual retry logic in search_bookdb(), but the old code (lines 149-171) is still present in the current file. This creates inconsistency - the PR shows it should be removed and replaced with the new centralized function. Verify the PR branch actually removes lines 149-171 and replaces with the new handle_rate_limit_response() call as shown in diff lines 199-210.
MEDIUM fingerprint.py:247-252 if response.status_code == 429: block calls handle_rate_limit_response() but ignores return value The function returns a dict with should_retry and wait_seconds, but fingerprint.py only calls it for side effects (circuit breaker update) and doesn't use the retry logic. This is intentional per the comment "fail fast, don't retry" but inconsistent with how bookdb.py uses it. Document this pattern - either add a comment explaining why fingerprint endpoints intentionally ignore retry logic, or extract just the circuit breaker logic into a separate function.
LOW rate_limiter.py:291 def handle_rate_limit_response(response, api_name, retry_count=0, max_retries=2): No type hints on new function while fingerprint.py uses comprehensive type hints Add type hints for consistency: def handle_rate_limit_response(response: requests.Response, api_name: str, retry_count: int = 0, max_retries: int = 2) -> Dict[str, Any]:

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key missing on GET /search requests causing 403 FIXED Headers with X-API-Key now added to all /search calls in app.py (lines 55-59, 116-120) and search_bookdb_api (lines 3055-3060)
#154 Client-side rate limiting and Retry-After support FIXED Centralized handle_rate_limit_response() function parses Retry-After, implements exponential backoff, integrates circuit breaker. Frontend displays warnings on all 3 pages (library, queue, history)

Scope Status: SCOPE_OK

📝 Documentation Check

  • CHANGELOG.md: ✅ Updated - Both issues documented with technical details
  • README.md: N/A - No user-visible features requiring README update

🎯 Verdict

REQUEST_CHANGES

Critical issue to fix:

  1. Verify bookdb.py changes - The diff shows old rate limit code (lines 149-171) should be REPLACED with the new handle_rate_limit_response() call, but I can only see the old code in the current file. This suggests either:

    • The PR branch correctly makes this change but I'm reviewing against develop (most likely)
    • The PR is incomplete and both old and new code exist (would cause duplicate retry logic)

    Action required: Confirm the PR branch actually removes the old manual retry code in search_bookdb() and replaces it with the centralized function as shown in diff lines 199-210.

Recommended improvements:
2. Add type hints to handle_rate_limit_response() for consistency with fingerprint.py
3. Add clarifying comment in fingerprint.py explaining why rate limit responses are logged but not retried (intentional "fail fast" for supplementary endpoints)

@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

PR #156 fixes missing API key authentication on /search requests (Issue #155) and implements centralized rate limit handling with exponential backoff (Issue #154).

Codebase Patterns I Verified

  • Logging: Uses logger = logging.getLogger(__name__) pattern consistently
  • Error handling: Uses except Exception as e: with logging (checked providers/*.py)
  • Secret management: Uses load_secrets() from library_manager/config.py
  • Type hints: Not consistently used on internal functions (per existing codebase)
  • Rate limiting: Existing code already uses rate_limit_wait() pattern

✅ Good

  • Comprehensive fix: Addresses both Issue fix: API key not sent on /search requests #155 (missing API key) and Issue feat: Add client-side rate limiting and Retry-After support #154 (rate limit handling) properly
  • Code deduplication: Excellent refactoring - consolidated duplicate rate limit logic from bookdb.py and fingerprint.py into reusable handle_rate_limit_response() function
  • Exponential backoff: Proper implementation with 30s/60s/120s progression, respects server's Retry-After header
  • Circuit breaker integration: Properly calls record_api_failure() to update circuit breaker state
  • Frontend feedback: Added rate limit warnings to library.html, queue.html, and history.html with countdown timers
  • Documentation: CHANGELOG.md properly updated with clear descriptions of both fixes
  • Import hygiene: Properly exports new functions via __all__ in both rate_limiter.py and init.py

🚨 Issues Found

Severity Location Issue Fix
HIGH app.py:3040 search_bookdb_api() calls load_secrets() but load_secrets is not imported into app.py Add from library_manager.config import load_secrets to imports at top of app.py
HIGH app.py:10951 Second occurrence - same function calls load_secrets() without import Same fix as above
MEDIUM app.py:3040-3048 Missing error handling on load_secrets() - could crash if secrets.json is malformed Wrap in try/except like: secrets = load_secrets() or {}
MEDIUM rate_limiter.py:313 Function handle_rate_limit_response imports time module but doesn't verify response is actually a requests.Response object Add docstring note or defensive check: if not hasattr(response, 'headers'): return {...}
LOW app.py:3078-3080 Comments still say "BookBucket API" instead of "Skaldleita API" in docstring Update docstring on line 3022 (already partially updated but inconsistent)

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key not sent on /search requests Fixed in both search_bookdb_api() (line 3040) and api_search_bookdb() (line 10951) - both now build headers with X-API-Key
#154 Rate limit handling and Retry-After support Fully implemented with handle_rate_limit_response() - exponential backoff, Retry-After parsing, circuit breaker, and frontend notifications

Scope Status: SCOPE_OK - Both issues fully addressed with proper implementation

📝 Documentation Check

  • CHANGELOG.md: ✅ Updated with clear descriptions of both fixes
  • README.md: ✅ Updated version badge to 0.9.0-beta.126
  • Version bump: ✅ Correctly incremented in app.py

🎯 Verdict

REQUEST_CHANGES - Fix these critical issues before merge:

  1. CRITICAL: Add missing import in app.py - load_secrets() is called but not imported (lines 3040, 10951)
  2. RECOMMENDED: Add error handling around load_secrets() calls to prevent crashes on malformed config

The core implementation is excellent - great refactoring and proper handling of both issues. However, the missing import will cause an immediate NameError crash when these code paths are executed. This must be fixed before shipping.

After fixing the import, this is a solid PR that properly addresses both linked issues with good code quality.

@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

PR #156 addresses two issues: (1) missing API key headers on /search requests causing 403 errors, and (2) implementing centralized rate limit handling with exponential backoff, circuit breaker integration, and frontend user feedback.

Codebase Patterns I Verified

  • Error Handling: Project uses try/except Exception as e with logging (verified in bookdb.py, fingerprint.py, ollama.py)
  • Type Hints: Used on functions in fingerprint.py and isbn_lookup.py but not consistently across all modules
  • Logging: Uses logger = logging.getLogger(__name__) pattern throughout
  • Rate Limiting: Existing pattern in bookdb.py uses manual retry logic with Retry-After parsing
  • Documentation: CHANGELOG.md gets updated for all notable changes

✅ Good

  • Centralized rate limit handling - handle_rate_limit_response() eliminates duplicate code across bookdb.py, fingerprint.py, and app.py
  • Exponential backoff - Proper 30s/60s/120s progression prevents hammering the API
  • Circuit breaker integration - Calls record_api_failure() to prevent runaway retry loops
  • Frontend feedback - All three templates (library, queue, history) show rate limit messages with retry countdown
  • Comprehensive scope - Addresses both issues feat: Add client-side rate limiting and Retry-After support #154 and fix: API key not sent on /search requests #155 completely
  • Auth headers added - All /search endpoints now include X-API-Key header
  • Documentation - CHANGELOG.md properly updated with issue references

🚨 Issues Found

Severity File:Line Exact Code Quote Issue Fix
MEDIUM app.py:3079 headers = get_signed_headers()
headers['X-API-Key'] = api_key
API key added to signed headers dict without checking if headers is None Add null check or ensure get_signed_headers() never returns None
MEDIUM app.py:10951 resp = requests.get(endpoint, params=params, headers=headers, timeout=60) No try/except around the request call in api_search_bookdb() Wrap in try/except requests.RequestException to prevent 500 errors on network failures
LOW rate_limiter.py:313 def handle_rate_limit_response(response, api_name: str, retry_count: int = 0, max_retries: int = 2) -> dict: Type hint uses lowercase dict instead of Dict from typing module Use Dict[str, Any] from typing for consistency with fingerprint.py patterns, or keep lowercase (Python 3.9+)
LOW fingerprint.py:272 rl = handle_rate_limit_response(response, 'bookdb') Function called without retry_count parameter, defaults to 0, but result is only used for logging Acceptable as-is, but could be more explicit: retry_count=0

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key not sent on /search requests Headers with X-API-Key added to both search_bookdb_api() (app.py:3079-3090) and api_search_bookdb() (app.py:10940-10951)
#154 Rate limit handling + Retry-After support Centralized in handle_rate_limit_response() with exponential backoff, circuit breaker, Retry-After parsing, and frontend notifications on all search pages

Scope Status: SCOPE_OK - Both issues fully addressed

📝 Documentation Check

  • CHANGELOG.md: ✅ Updated (lines 9-19) with proper issue references and detailed descriptions
  • README.md: ✅ Updated version badge from 0.9.0-beta.125 to 0.9.0-beta.126

🎯 Verdict

REQUEST_CHANGES

Items to fix:

  1. MEDIUM Priority: Add error handling around requests.get() in api_search_bookdb() at line ~10951 to prevent 500 errors on network failures
  2. MEDIUM Priority: Verify get_signed_headers() never returns None, or add null check before mutating in app.py:3079

The centralized rate limiting implementation is solid, but the missing error handling around the network request could cause user-facing errors. Once those two items are addressed, this is good to merge.

@bucket-agent
Copy link

bucket-agent bot commented Feb 16, 2026

🔍 Vibe Check Review

Context

This PR addresses two related issues: adding the missing X-API-Key header to /search requests (#155) and implementing centralized rate limit handling with exponential backoff (#154).

Codebase Patterns I Verified

I examined the existing codebase to understand patterns:

  • Error handling: Uses try/except with specific exception types, logs errors with logger.debug/info/warning
  • Rate limiting: Already uses rate_limit_wait() from library_manager.providers.rate_limiter
  • Circuit breaker: Existing pattern with record_api_failure(), record_api_success(), is_circuit_open()
  • Logging: Uses logger = logging.getLogger(__name__) pattern throughout
  • API headers: bookdb.py already has get_signed_headers() for HMAC signing
  • Type hints: Minimal usage in this codebase, primarily on new functions
  • Network calls: Existing code uses timeout parameter on all requests.get/post calls

✅ Good

  • Centralization: New handle_rate_limit_response() eliminates duplicated rate limit logic across app.py and bookdb.py
  • Exponential backoff: Smart fallback (30s, 60s, 120s) when Retry-After header is missing or malformed
  • Circuit breaker integration: Properly calls record_api_failure() and checks circuit state
  • Timeout protection: 300s cap prevents infinite waits from malicious/misconfigured servers
  • Frontend UX: Rate limit warnings shown to users with retry countdown
  • Defensive parsing: Handles ValueError when parsing Retry-After header
  • Documentation: CHANGELOG.md updated with clear explanations of both fixes

🚨 Issues Found

Severity Location Exact Code Quote Issue Fix
MEDIUM app.py:3077-3081 headers = get_signed_headers() or {} Missing API key in fallback path if get_signed_headers() returns None Change to headers = get_signed_headers() or {'X-API-Key': api_key} or always add API key after: headers['X-API-Key'] = api_key
LOW fingerprint.py:272 handle_rate_limit_response(response, 'bookdb') Passing response but not using return value - should fail fast with no retry Correct - code logs warning and returns None. This is intentional "fail fast" behavior per the comment on line 270-271. No fix needed.
LOW fingerprint.py:298 handle_rate_limit_response(response, 'bookdb') Same as above for narrator lookup Same - intentional fail-fast. No fix needed.

Update on MEDIUM issue after re-reading:

Looking at lines 3077-3081 in the diff:

secrets = load_secrets()
api_key = secrets.get('bookdb_api_key') or BOOKDB_PUBLIC_KEY
headers = get_signed_headers() or {}
headers['X-API-Key'] = api_key

This is CORRECT - the API key is added on line 3080 after creating headers. The or {} is a defensive fallback if get_signed_headers() returns None. No issue here.

📋 Scope Verification

Issue Problem Addressed? Notes
#155 API key not sent on /search requests Fixed in search_bookdb_api() and api_search_bookdb() - both now add X-API-Key header via get_signed_headers() + explicit header assignment
#154 Rate limit handling with Retry-After New handle_rate_limit_response() centralizes logic with exponential backoff, circuit breaker integration, and proper header parsing. Applied to bookdb.py, fingerprint.py. Frontend shows rate limit warnings.

Scope Status: SCOPE_OK

Both issues fully addressed:

📝 Documentation Check

  • CHANGELOG.md: ✅ Updated with clear descriptions of both fixes
  • README.md: ✅ Version bumped to 0.9.0-beta.126

🎯 Verdict

APPROVE

This is clean, defensive code that properly addresses both linked issues:

  1. API authentication is now consistent across all endpoints
  2. Rate limiting is centralized with proper exponential backoff
  3. Circuit breaker integration prevents thundering herd
  4. Frontend provides clear feedback to users
  5. Edge cases handled (malformed headers, circuit breaker tripped)

No blocking issues found. The code follows existing patterns, includes proper error handling, and improves maintainability by eliminating duplication.

@deucebucket deucebucket merged commit f5fc800 into develop Feb 16, 2026
3 checks passed
@deucebucket deucebucket deleted the fix/issue-155-api-key-and-rate-limiting branch February 16, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant