Skip to content

bug: 404 responses with empty body raise generic APIError instead of NotFoundError #118

@dougborg

Description

@dougborg

Description

When the StockTrim API returns a 404 status code with an empty response body, the unwrap() utility function raises a generic APIError instead of the more specific NotFoundError.

Current Behavior

# When API returns 404 with no body
locations = await client.locations.get_all()
# Raises: APIError("No parsed response data for status 404", 404)

Root Cause

In stocktrim_public_api_client/utils.py, the unwrap() function checks response.parsed is None before checking the status code:

def unwrap(response: Response[T], *, raise_on_error: bool = True) -> T | None:
    if response.parsed is None:  # Line 123
        if raise_on_error:
            raise APIError(  # Generic APIError instead of NotFoundError
                f"No parsed response data for status {response.status_code}",
                response.status_code,
            )
        return None
    
    # Status code check happens AFTER parsed check (line 142)
    if response.status_code >= 400:
        # This code never runs for 404s with empty body
        ...

Expected Behavior

404 responses should raise NotFoundError regardless of whether the response body is empty:

locations = await client.locations.get_all()
# Should raise: NotFoundError("...", 404)

Impact

  • Severity: Medium
  • User Experience: Confusing error messages
  • Code Impact: Callers can't catch NotFoundError specifically for 404s with empty bodies
  • API Consistency: Some 404s raise NotFoundError, others raise APIError

Affected Scenarios

Any API endpoint that returns 404 with an empty body, including:

  • GET /api/locations when no locations exist
  • GET /api/purchase-orders when no purchase orders exist
  • GET /api/sales-orders when no sales orders exist
  • Potentially other empty list endpoints

Proposed Fix

Check the status code before checking if parsed is None:

def unwrap(response: Response[T], *, raise_on_error: bool = True) -> T | None:
    # Check status code FIRST
    if response.status_code >= 400:
        if not raise_on_error:
            return None
            
        # Raise specific exception based on status code
        if response.status_code == 404:
            raise NotFoundError(
                "Resource not found" if response.parsed is None else ...,
                response.status_code
            )
        # ... other status codes
    
    # Then check parsed data
    if response.parsed is None:
        # This case shouldn't happen for successful responses
        # but handle it gracefully
        return None  # or raise for unexpected case

Testing

Should add test cases for:

  • 404 with empty body -> raises NotFoundError
  • 404 with error body -> raises NotFoundError with message
  • 404 with parsed data -> raises NotFoundError

Related Files

  • stocktrim_public_api_client/utils.py (lines 123-164)
  • All helper classes that use unwrap()

Workaround

Callers can catch both exceptions:

try:
    locations = await client.locations.get_all()
except (NotFoundError, APIError) as e:
    if e.status_code == 404:
        # No locations found
        locations = []

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions