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

OperationalError covers a vast swath of problems with no way to programmatically distinguish between then #682

Closed
sigmavirus24 opened this issue Feb 23, 2018 · 5 comments
Milestone

Comments

@sigmavirus24
Copy link

Problem

While using SQLAlchemy with psycopg2, when we catch OperationalErrors they can cover a wide variety of issues including (but not limited to) problems creating an index and the server disconnecting in the middle of execution of a query.

At the present time, the way we have to determine whether we can retry an operation is by checking if a string is in the error, e.g., "some string that may or may not be unique" in str(exc). This method of detecting which kind of OperationalError we're trying to handle is inefficient (searching for strings inside of other strings is slow) and could break if we upgrade to a new version of psycopg2 that changes an error message we're relying on.

Possible Solutions

Return Postgres codes as an attribute when present

In some cases, these exceptions have error codes that are returned by Postgres and could be used to identify them consistently.

Create extra subclasses of OperationalError to allow disambiguation

With this, people can catch more granular exceptions and handle those, e..g,

try:
    # code
except psycopg2.ServerDisconnected:
    raise RetryableException('server disconnected')
except psycopg2.IndexCreationFailed as exc:
    # conditional handling
except psycopg2.OperationalError as exc:
    log.error('psycopg2 encountered an operational error "%s"', strc(exc))
    raise
@dvarrazzo
Copy link
Member

Psycopg exception have a pgcode attribute which indeed contains the postgres code of the error. You don't have to check the string message, and it's actually a bad idea to do that because that string is subject to localization.

The idea of creating a subclass for every code is something I had in mind to do for quite some time. I shelved it under "things to do for psycopg3" for a long time but it's actually a backward-compatible change so we may implement it for 2.8 (I could do that if I have resources, otherwise help is welcome).

@dvarrazzo dvarrazzo added this to the psycopg 2.8 milestone Feb 25, 2018
@dvarrazzo
Copy link
Member

Note: this is being worked in https://github.com/psycopg/psycopg2/tree/errors-module

@stevenwinfield
Copy link

This is great news!

In some cases, particularly around authentication, pgcode is None and so there is no alternative (that I have found) to parsing the error message.

For example, I'm using this classmethod to convert an OperationalError's message into an enum value that I can use more easily:

    @classmethod
    def from_error_message(cls, msg):
        ''' Return the reason implied by the given error message '''
        msg = msg.rstrip()
        if msg == "fe_sendauth: no password supplied":
            return cls.no_password_supplied
        elif msg.startswith("FATAL:  password authentication failed for user"):
            return cls.incorrect_password_postgres
        elif msg == "ERROR:  Auth failed":
            return cls.incorrect_password_pgbouncer
        elif msg.startswith("FATAL:  role") and msg.endswith("does not exist"):
            return cls.unknown_user_postgres
        elif msg == "ERROR:  No such user":
            return cls.unknown_user_pgbouncer
        else:
            raise ValueError("Unknown login error: " + msg)

Will these pgcode-less errors also get their own OperationalError subclasses?

@dvarrazzo
Copy link
Member

@stevenwinfield No, we don't have plan to add other information other than what the server or the postgres client library provides us, only to make easier to access them.

I feel your pain though. You should ask the postgres developer to add such a code. Note that those messages are susceptible of localization, apart from being more likely than a code to change from one version to the other.

@dvarrazzo
Copy link
Member

Merged to master. Here is the module documentation. Still space for changes so feedback welcome.

jugmac00 pushed a commit to jugmac00/launchpad that referenced this issue Apr 14, 2022
This fixes building that wheel on Python 3.8.

psycopg2 now raises more specific exception types in various situations
(see psycopg/psycopg2#682), so some tests need
to be adjusted for that, and `lp.testing.pgsql.PgTestSetup` can take
advantage of it.
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

No branches or pull requests

3 participants