Skip to content

fix(snowflake): close() swallows a failing cursor reset - #22800

Merged
desertaxle merged 1 commit into
PrefectHQ:mainfrom
noron12234:fix/snowflake-close-swallows-error
Aug 10, 2026
Merged

fix(snowflake): close() swallows a failing cursor reset#22800
desertaxle merged 1 commit into
PrefectHQ:mainfrom
noron12234:fix/snowflake-close-swallows-error

Conversation

@noron12234

Copy link
Copy Markdown
Contributor

Problem

SnowflakeConnector.close() returns out of its finally block:

def close(self):
    try:
        self.reset_cursors()
    finally:
        if self._connection is None:
            self.logger.info("There was no connection open to be closed.")
            return                      # <-- discards the in-flight exception
        self._connection.close()
        ...

A return inside finally discards whatever exception was propagating. So if
reset_cursors() raises on a connector that never opened a connection — the
_connection is None path — the caller is told the close succeeded and the
original error is gone.

That is the exact shape that hides cursor-cleanup failures during teardown,
including inside __exit__, where the connector is closed on the way out of a
with block that may itself already be unwinding.

Fix

Replace the early return with an if/else so the finally block falls off
the end. Logged messages and the connection teardown are byte-for-byte the same;
the only behavioural change is that an exception from reset_cursors() now
reaches the caller.

Test

Added test_close_propagates_a_failing_cursor_reset next to the existing
test_close. It monkeypatches reset_cursors to raise and asserts the error
escapes close().

With the early return restored, it fails:

E       Failed: DID NOT RAISE RuntimeError

pytest src/integrations/prefect-snowflake/tests/test_database.py -k close
→ 2 passed. ruff format --check clean on both files.

SnowflakeConnector.close() calls reset_cursors() inside try and returns from
the finally block when there is no open connection. Returning out of a finally
discards the exception that was propagating, so a reset_cursors() failure on a
connector that never opened a connection is reported to the caller as a clean
close.

Replace the early return with an if/else so the block falls off the end and the
in-flight exception continues to propagate. The logged messages and the
connection teardown are unchanged.

Added a regression test; with the early return restored it fails with
'DID NOT RAISE RuntimeError'.

@desertaxle desertaxle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@desertaxle
desertaxle merged commit 7d377ad into PrefectHQ:main Aug 10, 2026
19 checks passed
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.

2 participants