fix: lead APIError's string form with the error message - #724
Merged
Vincent Biret (baywet) merged 4 commits intoSep 10, 2026
Merged
Conversation
str(APIError) started with a newline and printed message: None for every generated error model, ignoring the primary_message the generator emits, so any log line or error title built from the first line was empty. The first line now carries the message (primary_message, then message, then the class name), the status code and the error code; the nested error payload follows on a second line.
There was a problem hiding this comment.
🟢 Approval recommended
The formatting change is localized, matches the stated issue/expected behavior, and is covered by targeted new tests for the key scenarios.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Vincent Biret (baywet)
left a comment
Vincent Biret (baywet)
left a comment
Member
There was a problem hiding this comment.
Thanks for the contribution!
The base class has no notion of an error code; `error.code` is the OData error contract as Graph exposes it. The first line is now the message and the status only, the code stays visible on the error line.
…tattr lookups primary_message is a real property on the base class, returning message, that generated error models override; __str__ reads it plus the status code and no longer looks up error or primary_message by name.
|
Vincent Biret (baywet)
approved these changes
Sep 10, 2026
Vincent Biret (baywet)
left a comment
Vincent Biret (baywet)
left a comment
Member
There was a problem hiding this comment.
Thank you for making the changes!
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to APIError.__str__, aligns with the stated issue/intent, and includes targeted regression tests for the key behaviors.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
APIError.__str__returned a multi-line string that started with a newline and printedmessage: Nonefor every generated error model, because the generator never setsmessage; it emits aprimary_messageproperty that__str__ignored. Anything that titles an exception by its first line (Sentry does exactlyvalue.splitlines()[0], most log viewers do the same) showedODataError:with nothing after it, andlogging.error("...: %s", err)printed a blank line followed by an indented block.primary_messageis now a property onAPIErrorthat returnsmessage; generated error models already override it with the message from the API's error payload.__str__reads it (falling back to the class name) and appends the status code in parentheses. There is nogetattrlookup any more, and the nestederror:payload line is gone with it; a generated model can render its payload in its own__str__if wanted. For the throttling example from the issue:This matches the dotnet abstraction, where the generated error overrides
MessagewithError?.Message; Python cannot override themessagedataclass field with a property, so the override lives onprimary_message.Related Issue
Fixes #723
Notes
The
getattr(self, "error", None)guard from #133 is removed;__str__no longer reaches into subclass fields.Testing Instructions
cd packages/abstractions && pytest tests/test_api_error.py: a bare message, the status suffix, the class-name fallback,primary_messagedefaulting tomessage, a generated-style subclass overridingprimary_message, and that no variant starts with a blank line..pylintrcoption warnings.