Skip to content

Centralize the duplicated status-code-to-exception mapping in client.py #30

Description

@TimEvci

Summary

The status-code-to-exception mapping in client.py is written out three times. It should be a single module-level constant with a small helper.

Details

The literal mapping

{
    404: MoneybirdNotFoundError,
    422: MoneybirdValidationError,
    429: MoneybirdRateLimitError,
}.get(status_code, MoneybirdAPIError)

appears at moneysnake/client.py:137-141, 156-160, and 224-228 (the last inside http_post_file). http_post_file also reimplements the entire raise_for_status → log → dispatch sequence that make_request already contains.

Suggested fix

Hoist the mapping and add a helper:

_ERROR_CLASSES = {
    404: MoneybirdNotFoundError,
    422: MoneybirdValidationError,
    429: MoneybirdRateLimitError,
}

def _raise_api_error(response, method, path):
    cls = _ERROR_CLASSES.get(response.status_code, MoneybirdAPIError)
    raise cls(
        status_code=response.status_code,
        response_body=response.text,
        method=method,
        path=path,
    )

Have both make_request and http_post_file call the helper.

Note: one of the three copies is inside the unreachable post-loop block; removing that block (tracked separately) eliminates that copy independently.

Acceptance criteria

  • The mapping is defined once.
  • make_request and http_post_file raise identical exception types for the same status codes.
  • Existing exception tests pass unchanged.

Also: http_get_raw breaks the exception contract

http_get_raw (client.py:173-182) calls response.raise_for_status() and lets a raw httpx.HTTPStatusError propagate, unlike every other helper which raises a MoneybirdAPIError subclass. As a result SalesInvoice.download_pdf() and download_ubl() fail with a different exception type than the rest of the library, so callers catching MoneybirdError (or MoneybirdNotFoundError for a missing invoice) won't catch download failures. It also skips the retry/logging path.

As part of centralizing the mapping, route http_get_raw through the shared helper so a failed download raises the appropriate Moneybird*Error.

Retry decision (settled): raw downloads should not be retried — keep the current single-attempt behavior and only fix the exception mapping. Do not route http_get_raw through make_request's retry loop.

Additional acceptance criteria:

  • A 404/422/429/5xx on a download raises the matching Moneybird*Error.
  • A test asserts download_pdf() on a not-found invoice raises MoneybirdNotFoundError.

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 request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions