fix(sqlite): raise `ProgrammingError` when operating on a blob with a… #6286

ever0de · 2025-11-21T12:21:05Z

… closed connection,

Fixed #6285

reference:

Summary by CodeRabbit

New Features
- Added ability to check if a database connection is closed
Bug Fixes
- Database read operations now fail gracefully with error messaging when the connection is closed

_{✏️ Tip: You can customize this high-level summary in your review settings.}

… closed connection, Fixed RustPython#6285

coderabbitai · 2025-11-21T12:21:23Z

Walkthrough

The PR adds an is_closed() method to the Connection type and hardens Blob::read with a precondition check to raise a ProgrammingError when operating on a blob with a closed connection, aligning with CPython sqlite3 behavior.

Changes

Cohort / File(s)	Summary
Blob/Connection hardening `crates/stdlib/src/sqlite.rs`	Added `Connection::is_closed()` method to check if the underlying database is initialized and open. Enhanced `Blob::read()` to validate connection state before reading and raise `ProgrammingError` with message "Cannot operate on a closed database" if closed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Single file with localized, straightforward changes to two related methods
Simple conditional logic (checking Option state) with standard error handling
Direct correspondence to issue requirements with minimal surface area

Possibly related PRs

stdlib(sqlite): Raise ProgrammingError in closed Blob context manager #6041: Adds closed-state checks to Blob operations in sqlite.rs, addressing related connection validity concerns in blob context managers.
Fix sqlite Connection initialization check #6199: Modifies Connection internal db (Option) state handling and error paths, affecting the same underlying state-management mechanism.
feat(stdlib/sqlite): Implement slice assignment for Blob #6039: Implements additional Blob functionality in the same file, sharing the closed-connection validation context.

Suggested reviewers

youknowone

Poem

🐰 A rabbit's cheer for checks so keen,
When blobs on closed databases convene!
No silent fails, just errors bright,
Connection states now held so tight. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly refers to the main change: raising ProgrammingError when operating on a blob with a closed connection, which matches the primary objective of this PR.
Linked Issues check	✅ Passed	The PR addresses all coding requirements from issue #6285: adds is_closed() check to Connection, hardens Blob::read to detect closed connections, and raises ProgrammingError with the correct message.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the blob closed-connection handling issue; no unrelated modifications are present.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

Provide your own instructions using the high_level_summary_instructions setting.
Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

📝 Description — Summarize the main change in 50–60 words, explaining what was done.

📓 References — List relevant issues, discussions, documentation, or related PRs.

📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.

📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |

✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7ddcd2 and edc9f29.

⛔ Files ignored due to path filters (1)

Lib/test/test_sqlite3/test_dbapi.py is excluded by !Lib/**

📒 Files selected for processing (1)

crates/stdlib/src/sqlite.rs (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

GitHub Check: Check the WASM package and demo
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run tests under miri
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Check Rust code with clippy
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Ensure compilation on various targets
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: auto_format

🔇 Additional comments (1)

crates/stdlib/src/sqlite.rs (1)

1010-1012: LGTM! Clean helper method for connection state check.

The is_closed() method correctly checks the connection state by inspecting the db mutex. This provides a clear, reusable way to determine if the connection has been closed.

coderabbitai · 2025-11-21T12:24:54Z

crates/stdlib/src/sqlite.rs

+            if self.connection.is_closed() {
+                return Err(new_programming_error(
+                    vm,
+                    "Cannot operate on a closed database".to_owned(),
+                ));
+            }


⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Verify that other blob methods handle closed connections correctly.

The check for closed connections in read() is correct and matches the CPython behavior. However, other blob methods like write, tell, seek, and the subscript operations may also need this check to be consistent.

Run the following script to examine if other blob methods should also include the closed-connection check:

🏁 Script executed:

#!/bin/bash # Description: Search for all Blob methods to verify consistent closed-connection handling echo "=== Blob methods in the file ===" ast-grep --pattern $'impl Blob { $$$ fn $METHOD($$$) $$$ $$$ }' echo -e "\n=== Checking which methods call connection.db_lock or check connection state ===" rg -n -A5 -B2 'impl Blob' crates/stdlib/src/sqlite.rs | rg -A5 'fn (write|tell|seek|__enter__|__exit__|subscript|ass_subscript)'

Length of output: 114

The web search doesn't provide targeted information about the specific codebase. Let me generate new shell scripts to examine the Blob implementation directly:

🏁 Script executed:

#!/bin/bash # Description: Extract Blob impl block and examine all methods echo "=== Finding impl Blob block ===" rg -n 'impl Blob' crates/stdlib/src/sqlite.rs -A 300 echo -e "\n=== Counting methods in Blob impl ===" rg -n 'fn ' crates/stdlib/src/sqlite.rs | grep -A1 -B1 'Blob'

Length of output: 12999

🏁 Script executed:

#!/bin/bash # Search for CPython's blob.c file content from GitHub echo "=== Searching for CPython blob.c on GitHub ===" curl -s https://raw.githubusercontent.com/python/cpython/main/Modules/_sqlite/blob.c | head -400

Length of output: 9655

Add closed-connection check to other blob methods for consistency with CPython behavior.

CPython's check_blob() function checks connection status on ALL blob operations (read, write, seek, tell, enter, exit). The Rust implementation only added this check to read(). The following methods should include the same check before calling self.inner(vm)?:

write() (line 2207)

tell() (line 2222)

seek() (line 2227)

__enter__() (line 2261)

__exit__() (line 2267)

subscript() (line 2313)

ass_subscript() (line 2350)

Without these checks, operations on other methods could fail with unclear errors when the connection is closed, unlike CPython which provides a consistent error message.

🤖 Prompt for AI Agents

In crates/stdlib/src/sqlite.rs around lines 2176-2360 (affecting the methods at/near lines 2207, 2222, 2227, 2261, 2267, 2313, 2350), add the same closed-connection guard used at lines 2176-2181 to each listed blob method: before calling self.inner(vm)? in write(), tell(), seek(), __enter__(), __exit__(), subscript(), and ass_subscript(), check if self.connection.is_closed() and if so return Err(new_programming_error(vm, "Cannot operate on a closed database".to_owned())); this ensures consistent CPython-like error messages for all blob operations when the connection is closed.

@ever0de How do you think about this comment?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@youknowone Can I handle this in a separate issue?

ShaharNaveh · 2025-11-21T13:47:51Z

crates/stdlib/src/sqlite.rs

            length: OptionalArg<c_int>,
            vm: &VirtualMachine,
        ) -> PyResult<PyRef<PyBytes>> {
+            if self.connection.is_closed() {


Can you please put the link in the PR here? it will be easier to access rather than going to git blame -> PR -> links

Could you please clarify the context? I am currently unable to understand why it's required.

Not required. Just my personal preference

This isn’t the official stance of our project, but I’d like to share my personal view on the matter.

If the behavior is something that’s common sense and easily predictable, I don’t think we need to attach a reference. In my opinion, this change falls into that category. Before adding this code, it might have been unclear whether such a check was necessary, but once the code is added, it becomes obvious that it is.

Some behaviors are hard to infer from the code alone. In those cases, having a reference linked is helpful. Depending on the situation, when pointing to the exact code is useful, we link specific line numbers; when it’s more important to refer to the conceptual location, we just mention the function name instead.

youknowone

Thanks!

youknowone · 2025-11-22T11:41:02Z

crates/stdlib/src/sqlite.rs

            length: OptionalArg<c_int>,
            vm: &VirtualMachine,
        ) -> PyResult<PyRef<PyBytes>> {
+            if self.connection.is_closed() {


This isn’t the official stance of our project, but I’d like to share my personal view on the matter.

If the behavior is something that’s common sense and easily predictable, I don’t think we need to attach a reference. In my opinion, this change falls into that category. Before adding this code, it might have been unclear whether such a check was necessary, but once the code is added, it becomes obvious that it is.

Some behaviors are hard to infer from the code alone. In those cases, having a reference linked is helpful. Depending on the situation, when pointing to the exact code is useful, we link specific line numbers; when it’s more important to refer to the conceptual location, we just mention the function name instead.

youknowone · 2025-11-22T11:41:49Z

crates/stdlib/src/sqlite.rs

+            if self.connection.is_closed() {
+                return Err(new_programming_error(
+                    vm,
+                    "Cannot operate on a closed database".to_owned(),
+                ));
+            }


@ever0de How do you think about this comment?

fix(sqlite): raise ProgrammingError when operating on a blob with a…

edc9f29

… closed connection, Fixed RustPython#6285

coderabbitai bot reviewed Nov 21, 2025

View reviewed changes

ShaharNaveh reviewed Nov 21, 2025

View reviewed changes

ShaharNaveh approved these changes Nov 21, 2025

View reviewed changes

youknowone approved these changes Nov 22, 2025

View reviewed changes

youknowone merged commit 567fb4d into RustPython:main Nov 22, 2025
13 checks passed

coderabbitai bot mentioned this pull request Nov 22, 2025

Fix sqlite connection reinitialization #6288

Merged

ever0de mentioned this pull request Nov 22, 2025

sqlite: Raise ProgrammingError when operating on a closed connection in Blob methods #6289

Closed

coderabbitai bot mentioned this pull request Nov 22, 2025

Ensure sqlite blob methods respect closed connections #6290

Merged

fix(sqlite): raise ProgrammingError when operating on a blob with a… #6286

fix(sqlite): raise ProgrammingError when operating on a blob with a… #6286

Uh oh!

Conversation

ever0de commented Nov 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowone Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

ever0de Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

ever0de Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

youknowone Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

youknowone Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(sqlite): raise `ProgrammingError` when operating on a blob with a… #6286

fix(sqlite): raise `ProgrammingError` when operating on a blob with a… #6286

ever0de commented Nov 21, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 21, 2025 •

edited

Loading

coderabbitai bot Nov 21, 2025 •

edited

Loading

ever0de Nov 21, 2025 •

edited

Loading