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

return exceptions on Execute commands for postgresql (asyncpg driver) #1320

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

phenobarbital
Copy link
Owner

@phenobarbital phenobarbital commented Oct 24, 2024

Summary by Sourcery

Improve error handling in the asyncpg driver by returning specific exceptions for execute commands and logging warnings for these exceptions. Update the library version to 2.9.4.

Bug Fixes:

  • Return specific exceptions for execute commands in the asyncpg driver, including QueryCanceledError, StatementError, UniqueViolationError, ForeignKeyViolationError, and NotNullViolationError.

Enhancements:

  • Log warnings for specific exceptions in the asyncpg driver to aid in debugging.

Chores:

  • Update the version of the asyncdb library from 2.9.3 to 2.9.4.

Copy link
Contributor

sourcery-ai bot commented Oct 24, 2024

Reviewer's Guide by Sourcery

This PR enhances error handling in the PostgreSQL (asyncpg) driver by adding specific exception handling for common database errors. The changes affect the execute methods in both the main driver and cursor implementations, ensuring that certain database-specific errors are logged and re-raised rather than being wrapped in a generic ProviderError.

Updated class diagram for asyncpg driver error handling

classDiagram
    class AsyncPgDriver {
        +execute(sentence, *args)
        +execute_many(sentence, *args)
    }
    class pgCursor {
        +execute(sentence, *args, **kwargs)
        +execute_many(sentence, *args)
    }
    class QueryCanceledError
    class StatementError
    class UniqueViolationError
    class ForeignKeyViolationError
    class NotNullViolationError
    class InvalidSQLStatementNameError
    class PostgresSyntaxError
    class UndefinedColumnError
    class UndefinedTableError
    class DuplicateTableError
    class PostgresError
    class InterfaceError
    class ProviderError
    class InterfaceWarning

    AsyncPgDriver --> QueryCanceledError
    AsyncPgDriver --> StatementError
    AsyncPgDriver --> UniqueViolationError
    AsyncPgDriver --> ForeignKeyViolationError
    AsyncPgDriver --> NotNullViolationError
    AsyncPgDriver --> InterfaceError
    AsyncPgDriver --> ProviderError

    pgCursor --> QueryCanceledError
    pgCursor --> StatementError
    pgCursor --> UniqueViolationError
    pgCursor --> ForeignKeyViolationError
    pgCursor --> NotNullViolationError
    pgCursor --> InvalidSQLStatementNameError
    pgCursor --> PostgresSyntaxError
    pgCursor --> UndefinedColumnError
    pgCursor --> UndefinedTableError
    pgCursor --> DuplicateTableError
    pgCursor --> PostgresError
    pgCursor --> InterfaceWarning
    pgCursor --> ProviderError

    note for AsyncPgDriver "Handles specific database errors and logs them"
    note for pgCursor "Handles specific database errors and logs them"
Loading

File-Level Changes

Change Details Files
Enhanced error handling for PostgreSQL-specific exceptions
  • Added specific exception handling for QueryCanceledError, StatementError, UniqueViolationError, ForeignKeyViolationError, and NotNullViolationError
  • Added warning log messages for caught database-specific exceptions
  • Modified error handling to re-raise database-specific exceptions instead of wrapping them
  • Simplified error messages by directly using the error object instead of wrapping it in a string
asyncdb/drivers/pg.py
Version bump for the package
  • Incremented version from 2.9.3 to 2.9.4
asyncdb/version.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@phenobarbital phenobarbital merged commit 17f48c0 into master Oct 24, 2024
1 of 2 checks passed
@phenobarbital phenobarbital deleted the new-drivers branch October 24, 2024 11:20
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @phenobarbital - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider extracting the common exception tuple (QueryCanceledError, StatementError, etc.) into a module-level constant to avoid repetition and make future maintenance easier.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -414,10 +417,25 @@ async def execute(self, sentence, *args):
"""
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider extracting duplicated error handling logic into a reusable decorator function

The error handling logic is duplicated across execute(), execute_many() and the pool's execute(). Consider extracting this into a decorator:

def handle_pg_errors(f):
    async def wrapper(*args, **kwargs):
        try:
            return await f(*args, **kwargs)
        except (
            QueryCanceledError,
            StatementError, 
            UniqueViolationError,
            ForeignKeyViolationError,
            NotNullViolationError
        ) as err:
            self._logger.warning(f"AsyncPg: {err}")
            raise
        except (
            InvalidSQLStatementNameError,
            PostgresSyntaxError, 
            UndefinedColumnError,
            UndefinedTableError
        ) as err:
            return await self._serializer(None, err)
        except Exception as err:
            return await self._serializer(None, f"Error: {err}")
    return wrapper

@handle_pg_errors
async def execute(self, sentence, *args, **kwargs):
    self.start_timing()
    await self.valid_operation(sentence)
    self._result = await self._connection.execute(sentence, *args, **kwargs)
    self.generated_at()
    return await self._serializer(self._result, None)

This maintains the improved error handling while eliminating duplication. The decorator can be reused across all execute methods.

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.

1 participant