Skip to content

fix: raise AttributeError instead of KeyError in __getattr__#123

Open
AmSach wants to merge 1 commit into
dashscope:mainfrom
AmSach:fix/attributeerror-on-missing-key
Open

fix: raise AttributeError instead of KeyError in __getattr__#123
AmSach wants to merge 1 commit into
dashscope:mainfrom
AmSach:fix/attributeerror-on-missing-key

Conversation

@AmSach
Copy link
Copy Markdown

@AmSach AmSach commented May 7, 2026

Fixed the bug described in issue #114. Here's what was wrong and how I fixed it:

Problem

DashScopeAPIResponse.getattr raised KeyError when an attribute was missing, but Python's attribute-access protocol requires AttributeError. This broke common Python patterns like getattr(obj, name, default) and hasattr(obj, name) — both rely on AttributeError to trigger the default/fallback path.

Fix

Changed getattr in dashscope/api_entities/dashscope_response.py to catch the KeyError and re-raise it as AttributeError:

def __getattr__(self, attr):
    try:
        return self[attr]
    except KeyError:
        raise AttributeError(attr) from None

Tested by

All 4 test cases pass:

  • getattr(response, 'missing', 'default') → returns 'default' ✓
  • hasattr(response, 'missing') → returns False ✓
  • response.existing_attr → still works correctly ✓
  • response.missing_attr → raises AttributeError (correct) ✓

Fixes: #114

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the getattr method in dashscope/api_entities/dashscope_response.py to handle missing keys by raising an AttributeError instead of a KeyError. The reviewer suggests enhancing the error message to include the class name, which would improve debugging clarity and align with standard Python error reporting.

try:
return self[attr]
except KeyError:
raise AttributeError(attr) from None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

While raising AttributeError correctly fixes the reported issue, the error message could be improved to follow standard Python conventions. Currently, it only provides the attribute name, which results in a terse error like AttributeError: missing_attr. A more descriptive message including the class name (e.g., AttributeError: 'DashScopeAPIResponse' object has no attribute 'missing_attr') would be more helpful for debugging and consistent with built-in Python behavior.

Suggested change
raise AttributeError(attr) from None
raise AttributeError(f"'{type(self).__name__}' object has no attribute '{attr}'") from None

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.

DashScopeAPIResponse.__getattr__ raises KeyError instead of AttributeError

1 participant