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

SNOW-897499: Throw ReauthenticationRequest error instead of ProgrammingError #1705

Open
vgutkovsk opened this issue Aug 22, 2023 · 1 comment
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team

Comments

@vgutkovsk
Copy link

vgutkovsk commented Aug 22, 2023

What is the current behavior?

Currently if ReauthenticationRequest gets raised, it gets caught by the library and instead ProgrammingError gets raised.
network.py

except ReauthenticationRequest as ex:
    if self._connection._authenticator != EXTERNAL_BROWSER_AUTHENTICATOR:
        raise ex.cause
    ret = self._connection._reauthenticate()

Both errors would be captured in logs, but there's no way to determine in code if the error is caused by reauthentication request.

What is the desired behavior?

Desired behaviour is either raising ProgrammingError but like this: raise ex.cause from ex. Or throw ReauthenticationRequest error instead.

How would this improve snowflake-connector-python?

This would allow users to catch ReauthenticationRequest separately from ProgrammingError and have a different logic to handle one or the other.

References and other background

I have this code for running queries in Snowflake.

try:
    snowflake.execute(query)
except (snowflake.connector.InterfaceError,
        snowflake.connector.OperationalError) as exc: # these errors can be handled by recreating connection
    logging.error(exc)
    snowflake.recreate_connection()
    snowflake.execute(query)

But unfortunately this code fails when ProgrammingError gets raised. I can see that most of the times ProgrammingError gets raised in situations when recreating connection would not help. However, it's not the case when it gets raised because of ReauthenticationRequest. To handle this I need to do the following:

try:
    snowflake.execute(query)
except (snowflake.connector.InterfaceError,
        snowflake.connector.OperationalError,
        snowflake.connector.errors.ProgrammingError) as exc:
    if isinstance(snowflake.connector.errors.ProgrammingError, exc) and 'Authentication token has expired' not in exc.msg:
        raise
    logging.error(exc)
    snowflake.recreate_connection()
    snowflake.execute(query)

This code is not very stable because it relies on the message in the error.

@github-actions github-actions bot changed the title Throw ReauthenticationRequest error instead of ProgrammingError SNOW-897499: Throw ReauthenticationRequest error instead of ProgrammingError Aug 22, 2023
@sfc-gh-dszmolka
Copy link

hi and thank you for raising this request! we'll consider it as a possible future improvement and of course if you have the capacity to help out and submit a PR, that would be also appreciated!

@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage_done Initial triage done, will be further handled by the driver team and removed needs triage labels Mar 11, 2024
@sfc-gh-ashahi sfc-gh-ashahi self-assigned this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

4 participants