Skip to content

fix: avoid masking query errors after connection close - #455

Closed
dajiaohuang wants to merge 1 commit into
albertodonato:mainfrom
dajiaohuang:fix-latency-after-connection-close
Closed

dajiaohuang wants to merge 1 commit into
albertodonato:mainfrom
dajiaohuang:fix-latency-after-connection-close

Conversation

@dajiaohuang

Copy link
Copy Markdown

Closes the KeyError path described in #449: the connection close handler clears conn.info, while after_cursor_execute may still run and unconditionally pop query_start_time. When it is absent, skip recording latency rather than raising a new exception that masks the original query failure.

The regression test dispatches the real SQLAlchemy before/close/after events on a synthetic connection. Normal in-memory SQLite query latency remains covered by the existing database tests.

This addresses the reported exception-masking defect only; it does not establish or claim a fix for the unconfirmed SIGSEGV. The open #451 event-loop invalidation change is separate; current main already dispatches invalidation through its timeout executor.

Validation: tests/db_test.py -k 'not test_connect_error' passed (46); targeted regression passed; Ruff check/format and git diff --check passed. Full unit suite on Windows had two unrelated failures in tests that expect sqlite:////invalid to fail; Windows SQLite accepts that path. The rest passed (233).

@albertodonato

Copy link
Copy Markdown
Owner

Thanks for the PR, but the fix there only avoid they KeyError in case the latency is not there.
This happens because the connection info can get cleared before the info is accessed.
#456 should avoid this entirely

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