Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve error message for _sort_summary_list failures #243

Merged
merged 1 commit into from Apr 5, 2024

Conversation

mansenfranzen
Copy link
Owner

@mansenfranzen mansenfranzen commented Apr 5, 2024

Type

bug_fix


Description

  • Enhanced error handling in _sort_summary_list to provide more informative error messages.
  • The error message now includes the model name and sort order when a sorting exception occurs.

Changes walkthrough

Relevant files
Error handling
autodocumenters.py
Enhanced Error Handling and Messaging in `_sort_summary_list`

sphinxcontrib/autodoc_pydantic/directives/autodocumenters.py

  • Improved error handling in _sort_summary_list by adding a try-except
    block.
  • Enhanced the error message for exceptions during sorting, including
    the model name and sort order.
  • +7/-7     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    github-actions bot commented Apr 5, 2024

    PR Description updated to latest commit (055db00)

    Copy link
    Contributor

    github-actions bot commented Apr 5, 2024

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are localized to a specific function within a single file, focusing on error handling improvements. The logic seems straightforward, but understanding the context of the error handling and ensuring the new messages are clear and useful requires some domain knowledge.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The error message construction in the except block may miss a space between 'model' and the model name ({self.object_name}), which could lead to a concatenated string that is hard to read.

    🔒 Security concerns

    No

    Code feedback:
    relevant filesphinxcontrib/autodoc_pydantic/directives/autodocumenters.py
    suggestion      

    Consider adding a space before {self.object_name} in the error message to ensure proper readability of the output. [important]

    relevant linef'{self.object_name} with sort order {sort_order}.'


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    github-actions bot commented Apr 5, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a space for readability in the error message.

    The error message for the TypeError exception does not include a space between 'model' and
    the model name (self.object_name). Adding a space will improve the readability of the
    error message.

    sphinxcontrib/autodoc_pydantic/directives/autodocumenters.py [577-578]

    -f'Uncaught exception while sorting fields for model'
    +f'Uncaught exception while sorting fields for model '
     f'{self.object_name} with sort order {sort_order}.'
     
    Include the original exception message for more detailed error context.

    To provide more context in the error message when a TypeError is raised during sorting,
    it's helpful to include the original exception message (e) along with the custom error
    message. This can aid in debugging by providing more detailed information about the root
    cause of the error.

    sphinxcontrib/autodoc_pydantic/directives/autodocumenters.py [576-580]

     msg = (
         f'Uncaught exception while sorting fields for model '
    -    f'{self.object_name} with sort order {sort_order}.'
    +    f'{self.object_name} with sort order {sort_order}. Exception: {e}'
     )
     raise ValueError(msg).with_traceback(e.__traceback__) from e
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @codecov-commenter
    Copy link

    codecov-commenter commented Apr 5, 2024

    Codecov Report

    Attention: Patch coverage is 50.00000% with 2 lines in your changes are missing coverage. Please review.

    Project coverage is 94.26%. Comparing base (461be30) to head (055db00).

    Files Patch % Lines
    ...rib/autodoc_pydantic/directives/autodocumenters.py 50.00% 2 Missing ⚠️
    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main     #243      +/-   ##
    ==========================================
    - Coverage   94.34%   94.26%   -0.08%     
    ==========================================
      Files          12       12              
      Lines        1114     1116       +2     
    ==========================================
    + Hits         1051     1052       +1     
    - Misses         63       64       +1     

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @mansenfranzen mansenfranzen merged commit 87923a1 into main Apr 5, 2024
    35 of 37 checks passed
    @mansenfranzen mansenfranzen deleted the fix_sort_order_error branch April 5, 2024 12:34
    @github-actions github-actions bot mentioned this pull request Apr 5, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants