Skip to content

Add explicit decision/fallback logging in Book.from_isbn - #106

Closed
User0332 with Copilot wants to merge 3 commits into
mainfrom
copilot/add-logging-to-book-from-isbn
Closed

Add explicit decision/fallback logging in Book.from_isbn#106
User0332 with Copilot wants to merge 3 commits into
mainfrom
copilot/add-logging-to-book-from-isbn

Conversation

Copilot AI commented May 29, 2026

Copy link
Copy Markdown

Book.from_isbn lacked visibility into fallback/error branches, making ISBN lookup/debug flows opaque in production logs. This update adds explicit logs for branch decisions and fallback failures while preserving existing behavior.

  • Logging coverage in Book.from_isbn

    • Added an info log when an ISBN-10 input is converted to ISBN-13.
    • Added warning logs when Google Books metadata is missing and execution falls back to Open Library.
    • Added warning logs for Open Library cover lookup failure paths (error status and request exception).
  • Log safety hardening

    • Sanitized ISBN values used in the new log lines by escaping CR/LF characters before interpolation to reduce log-injection risk.
  • Representative snippet

    def sanitize_log_value(value: str) -> str:
        return value.replace("\r", "\\r").replace("\n", "\\n")
    
    safe_isbn = sanitize_log_value(isbn)
    
    if isbnlib.is_isbn10(isbn):
        logger.info(f"Decision: from-isbn - Converting ISBN-10 to ISBN-13. isbn={safe_isbn}")
        isbn = isbnlib.to_isbn13(isbn)
        safe_isbn = sanitize_log_value(isbn)
    
    ...
    except KeyError:
        logger.warning(f"Failure: from-isbn - Google Books lookup missing data. isbn={safe_isbn}. Falling back to Open Library")

Copilot AI linked an issue May 29, 2026 that may be closed by this pull request
@User0332
User0332 marked this pull request as ready for review May 29, 2026 04:10
Copilot AI changed the title [WIP] Add logging to the Book.from_isbn method Add explicit decision/fallback logging in Book.from_isbn May 29, 2026
Copilot AI requested a review from User0332 May 29, 2026 04:11
Comment thread website/app.py

if isbnlib.is_isbn10(isbn):
logger.info(f"Decision: from-isbn - Converting ISBN-10 to ISBN-13. isbn={isbn}")
logger.info(f"Decision: from-isbn - Converting ISBN-10 to ISBN-13. isbn={safe_isbn}")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove "decision" from the string

Comment thread website/app.py

@staticmethod
def from_isbn(isbn: str) -> "Book":
def sanitize_log_value(value: str) -> str:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete this, we don't need to sanitize the ISBN

Copilot AI requested a review from User0332 May 29, 2026 04:14
@User0332 User0332 closed this May 29, 2026
@User0332
User0332 deleted the copilot/add-logging-to-book-from-isbn branch May 29, 2026 04:15
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.

Add logging to the Book.from_isbn method

2 participants