Skip to content

Drop redundant exception interpolation in logger.exception(...) calls #647

Description

@nanotaboada

Problem

services/player_service.py logs caught exceptions like this (three
occurrences, in create_async, update_by_squad_number_async,
delete_by_squad_number_async):

except SQLAlchemyError as error:
    logger.exception("Error trying to create the Player: %s", error)
    await async_session.rollback()

Logger.exception() already attaches the active exception — type, message, and
full traceback — to the log record. Interpolating error with %s duplicates
the message text and adds noise. The idiomatic call passes only the static
message.

Proposed Solution

except SQLAlchemyError:
    logger.exception("Error trying to create the Player")
    await async_session.rollback()
    return None

The logger.error("Player not found for update: squad_number=%s", squad_number) calls are correct as-is — there is no active exception
there, and they interpolate data, not an exception.

Suggested Approach

  1. In each except SQLAlchemyError as error: block that calls
    logger.exception(...), drop the as error binding and the : %s /
    , error argument.
  2. Leave the logger.error(...) data-interpolation calls untouched.
  3. tools/seed_*.py has the same logger.exception("...: %s", exc) pattern —
    fix there too if convenient (lower priority; tools/ is legacy).
  4. Run uv run pytest.

Acceptance Criteria

  • logger.exception() calls carry a static message, no exception %s
  • Unused as error bindings removed
  • logger.error(...) data-interpolation calls unchanged
  • All tests pass
  • CHANGELOG.md updated

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestpriority:lowNice-to-have improvement. Can be deferred without blocking other work.pythonPull requests that update Python codepython:idiomsRefactors toward idiomatic Python (PEP conventions, stdlib idioms)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions