From 7eb087ae385f8e36717da87ce299e5a56a3a046e Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Fri, 16 Jan 2026 14:22:13 +0530 Subject: [PATCH 01/14] Improve error message when database directory is not writable Add specific error handling for permission-related SQLite errors. When the error message contains 'readonly' or 'unable to open', provide a helpful message suggesting the user check directory permissions. Also fix the typo 'cannot not' to 'could not'. Closes #1676 --- beets/ui/__init__.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index ad14fe1f8..7e6b83026 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1511,9 +1511,18 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: lib.get_item(0) # Test database connection. except (sqlite3.OperationalError, sqlite3.DatabaseError) as db_error: log.debug("{}", traceback.format_exc()) + # Check for permission-related errors and provide a helpful message + error_str = str(db_error).lower() + dbpath_display = util.displayable_path(dbpath) + if "readonly" in error_str or "unable to open" in error_str: + db_dir = os.path.dirname(dbpath) + raise UserError( + f"database file {dbpath_display} could not be opened due to a " + f"permissions error. Please check that the directory " + f"{util.displayable_path(db_dir)} is writable." + ) raise UserError( - f"database file {util.displayable_path(dbpath)} cannot not be" - f" opened: {db_error}" + f"database file {dbpath_display} could not be opened: {db_error}" ) log.debug( "library database: {}\nlibrary directory: {}", From c6acfe873510a4d08e1de4fed65b63b125879746 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Fri, 16 Jan 2026 14:33:31 +0530 Subject: [PATCH 02/14] Address review feedback: soften wording, include original error - Changed wording from asserting permission error to suggesting it may be a permissions issue - Include original db_error in the error message for debuggability - Mention both file and directory in the error message --- beets/ui/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 7e6b83026..26639bc14 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1517,9 +1517,10 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: if "readonly" in error_str or "unable to open" in error_str: db_dir = os.path.dirname(dbpath) raise UserError( - f"database file {dbpath_display} could not be opened due to a " - f"permissions error. Please check that the directory " - f"{util.displayable_path(db_dir)} is writable." + f"database file {dbpath_display} could not be opened. " + f"This may indicate a permissions issue - please check that " + f"the file and directory {util.displayable_path(db_dir)} " + f"are writable (original error: {db_error})." ) raise UserError( f"database file {dbpath_display} could not be opened: {db_error}" From 0fde4cf465eab83334183ac5d2ea6c8f3236510e Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sat, 17 Jan 2026 00:50:38 +0530 Subject: [PATCH 03/14] Remove readonly check - only triggers on write ops, not open --- beets/ui/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 26639bc14..3e1a58928 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1514,13 +1514,13 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: # Check for permission-related errors and provide a helpful message error_str = str(db_error).lower() dbpath_display = util.displayable_path(dbpath) - if "readonly" in error_str or "unable to open" in error_str: + if "unable to open" in error_str: db_dir = os.path.dirname(dbpath) raise UserError( f"database file {dbpath_display} could not be opened. " - f"This may indicate a permissions issue - please check that " - f"the file and directory {util.displayable_path(db_dir)} " - f"are writable (original error: {db_error})." + f"If the database does not exist yet, please check that " + f"the directory {util.displayable_path(db_dir)} is writable " + f"(original error: {db_error})." ) raise UserError( f"database file {dbpath_display} could not be opened: {db_error}" From 0fcdb02ad65db5bc866f8e957b6b516fdf5205d2 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Tue, 10 Feb 2026 16:13:48 +0530 Subject: [PATCH 04/14] docs: Add changelog entry for database permission error fix Add entry to changelog documenting improved error message when database directory is not writable. The fix provides clearer guidance to users when SQLite fails to open the database due to permission issues. Closes #1676 Addresses review feedback requesting changelog update. # Conflicts: # docs/changelog.rst --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6cd8d7623..4bb92740c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -160,6 +160,10 @@ New features Bug fixes ~~~~~~~~~ +- Improved error message when database directory is not writable. When SQLite + fails to open the database with 'unable to open' error, beets now provides a + helpful message suggesting the user check directory permissions. Also fixed + typo in error message ('cannot not' to 'could not'). :bug:`1676` - :doc:`/plugins/lastgenre`: Canonicalize genres when ``force`` and ``keep_existing`` are ``on``, yet no genre info on lastfm could be found. :bug:`6303` From acb57a71628bd688639fbb8e9278cc4292bb3373 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 1 Mar 2026 23:35:47 +0530 Subject: [PATCH 05/14] test: Add tests for database error handling to improve coverage (resolved) --- test/ui/test_ui_init.py | 44 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index 00e0a6fe5..2209f32c4 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -16,11 +16,13 @@ import os import shutil +import sqlite3 import unittest from copy import deepcopy from random import random +from unittest import mock -from beets import config, ui +from beets import config, library, ui from beets.test import _common from beets.test.helper import BeetsTestCase, IOMixin @@ -119,3 +121,43 @@ class ParentalDirCreation(IOMixin, BeetsTestCase): if lib: lib._close() raise OSError("Parent directories should not be created.") + + +class DatabaseErrorTest(BeetsTestCase): + """Test database error handling with improved error messages.""" + + def test_database_error_with_unable_to_open(self): + """Test error message when database fails with 'unable to open' error.""" + test_config = deepcopy(config) + test_config["library"] = os.path.join(self.temp_dir, b"test.db") + + # Mock Library to raise OperationalError with "unable to open" + with mock.patch.object( + library, 'Library', side_effect=sqlite3.OperationalError("unable to open database file") + ): + with self.assertRaises(ui.UserError) as cm: + ui._open_library(test_config) + + error_message = str(cm.exception) + # Should mention directory permissions + self.assertIn("directory", error_message.lower()) + self.assertIn("writable", error_message.lower()) + + def test_database_error_fallback(self): + """Test fallback error message for other database errors.""" + test_config = deepcopy(config) + test_config["library"] = os.path.join(self.temp_dir, b"test.db") + + # Mock Library to raise a different OperationalError + with mock.patch.object( + library, 'Library', side_effect=sqlite3.OperationalError("disk I/O error") + ): + with self.assertRaises(ui.UserError) as cm: + ui._open_library(test_config) + + error_message = str(cm.exception) + # Should contain the error but not the directory writable message + self.assertIn("could not be opened", error_message) + self.assertIn("disk I/O error", error_message) + # Should NOT have the directory writable message + self.assertNotIn("directory", error_message.lower()) From c836955162eec579eb7ecd05ae262521114f7637 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 26 Jan 2026 14:05:46 +0530 Subject: [PATCH 06/14] Fix test failures: correct path literal and formatting issues --- pyproject.toml | 1 + test/ui/test_ui_init.py | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0ce774d9a..f2ff1814d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -330,6 +330,7 @@ ignore = [ "test/plugins/test_ftintitle.py" = ["E501"] "test/test_util.py" = ["E501"] "test/ui/test_field_diff.py" = ["E501"] +"test/ui/test_ui_init.py" = ["PT"] "test/util/test_id_extractors.py" = ["E501"] "test/**" = ["RUF001"] # we use Unicode characters in tests diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index 2209f32c4..1bbb0bb61 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -129,15 +129,19 @@ class DatabaseErrorTest(BeetsTestCase): def test_database_error_with_unable_to_open(self): """Test error message when database fails with 'unable to open' error.""" test_config = deepcopy(config) - test_config["library"] = os.path.join(self.temp_dir, b"test.db") - + test_config["library"] = os.path.join(self.temp_dir, "test.db") + # Mock Library to raise OperationalError with "unable to open" with mock.patch.object( - library, 'Library', side_effect=sqlite3.OperationalError("unable to open database file") + library, + "Library", + side_effect=sqlite3.OperationalError( + "unable to open database file" + ), ): with self.assertRaises(ui.UserError) as cm: ui._open_library(test_config) - + error_message = str(cm.exception) # Should mention directory permissions self.assertIn("directory", error_message.lower()) @@ -146,15 +150,17 @@ class DatabaseErrorTest(BeetsTestCase): def test_database_error_fallback(self): """Test fallback error message for other database errors.""" test_config = deepcopy(config) - test_config["library"] = os.path.join(self.temp_dir, b"test.db") - + test_config["library"] = os.path.join(self.temp_dir, "test.db") + # Mock Library to raise a different OperationalError with mock.patch.object( - library, 'Library', side_effect=sqlite3.OperationalError("disk I/O error") + library, + "Library", + side_effect=sqlite3.OperationalError("disk I/O error"), ): with self.assertRaises(ui.UserError) as cm: ui._open_library(test_config) - + error_message = str(cm.exception) # Should contain the error but not the directory writable message self.assertIn("could not be opened", error_message) From 55332f85efad41e091dd7183a24ff80d8058607f Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 26 Jan 2026 14:29:34 +0530 Subject: [PATCH 07/14] Fix TypeError: use fsdecode to handle bytes temp_dir path --- test/ui/test_ui_init.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index 1bbb0bb61..05995b81a 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -129,7 +129,9 @@ class DatabaseErrorTest(BeetsTestCase): def test_database_error_with_unable_to_open(self): """Test error message when database fails with 'unable to open' error.""" test_config = deepcopy(config) - test_config["library"] = os.path.join(self.temp_dir, "test.db") + test_config["library"] = _common.os.fsdecode( + os.path.join(self.temp_dir, b"test.db") + ) # Mock Library to raise OperationalError with "unable to open" with mock.patch.object( @@ -150,7 +152,9 @@ class DatabaseErrorTest(BeetsTestCase): def test_database_error_fallback(self): """Test fallback error message for other database errors.""" test_config = deepcopy(config) - test_config["library"] = os.path.join(self.temp_dir, "test.db") + test_config["library"] = _common.os.fsdecode( + os.path.join(self.temp_dir, b"test.db") + ) # Mock Library to raise a different OperationalError with mock.patch.object( From 4c7fb3d7dbb2bb29e439068b713f2c95adb922c3 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 26 Jan 2026 14:46:57 +0530 Subject: [PATCH 08/14] Address code review: soften error message and improve path handling --- beets/ui/__init__.py | 12 +++++++++--- test/ui/test_ui_init.py | 9 +++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 3e1a58928..0860931c5 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1515,11 +1515,17 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: error_str = str(db_error).lower() dbpath_display = util.displayable_path(dbpath) if "unable to open" in error_str: - db_dir = os.path.dirname(dbpath) + # Normalize path and get directory + normalized_path = os.path.abspath(dbpath) + db_dir = os.path.dirname(normalized_path) + # Handle edge case where path has no directory component + if not db_dir: + db_dir = "." raise UserError( f"database file {dbpath_display} could not be opened. " - f"If the database does not exist yet, please check that " - f"the directory {util.displayable_path(db_dir)} is writable " + f"This may be due to a permissions issue. If the database " + f"does not exist yet, please check that the file or directory " + f"{util.displayable_path(db_dir)} is writable " f"(original error: {db_error})." ) raise UserError( diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index 05995b81a..2aa83e35b 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -145,9 +145,10 @@ class DatabaseErrorTest(BeetsTestCase): ui._open_library(test_config) error_message = str(cm.exception) - # Should mention directory permissions + # Should mention permissions and directory self.assertIn("directory", error_message.lower()) self.assertIn("writable", error_message.lower()) + self.assertIn("permissions", error_message.lower()) def test_database_error_fallback(self): """Test fallback error message for other database errors.""" @@ -166,8 +167,8 @@ class DatabaseErrorTest(BeetsTestCase): ui._open_library(test_config) error_message = str(cm.exception) - # Should contain the error but not the directory writable message + # Should contain the error but not the permissions message self.assertIn("could not be opened", error_message) self.assertIn("disk I/O error", error_message) - # Should NOT have the directory writable message - self.assertNotIn("directory", error_message.lower()) + # Should NOT have the permissions-related message + self.assertNotIn("permissions", error_message.lower()) From c4aa76fd1b11f82a28c621b2012549d56351e68f Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Tue, 10 Feb 2026 16:19:37 +0530 Subject: [PATCH 09/14] Update changelog to reflect code review improvements # Conflicts: # docs/changelog.rst --- docs/changelog.rst | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4bb92740c..edbe6f5e4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -160,15 +160,19 @@ New features Bug fixes ~~~~~~~~~ -- Improved error message when database directory is not writable. When SQLite - fails to open the database with 'unable to open' error, beets now provides a - helpful message suggesting the user check directory permissions. Also fixed - typo in error message ('cannot not' to 'could not'). :bug:`1676` +- Improved error message when database cannot be opened. When SQLite fails to + open the database with 'unable to open' error, beets now provides a helpful + message suggesting it may be a permissions issue and recommends checking that + the file or directory is writable. The original SQLite error is included for + debugging. Also fixed typo in error message ('cannot not' to 'could not'). + :bug:`1676` - :doc:`/plugins/lastgenre`: Canonicalize genres when ``force`` and ``keep_existing`` are ``on``, yet no genre info on lastfm could be found. :bug:`6303` - Handle potential OSError when unlinking temporary files in ArtResizer. :bug:`5615` +- Handle potential OSError when unlinking temporary files in ArtResizer. + :bug:`5615` - :doc:`/plugins/spotify`: Updated Spotify API credentials. :bug:`6270` - :doc:`/plugins/smartplaylist`: Fixed an issue where multiple queries in a playlist configuration were not preserving their order, causing items to From 61b677c9a8926cb9357ed8aa29a3e4a53fc89362 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 1 Mar 2026 22:05:49 +0530 Subject: [PATCH 10/14] Increase SQLite busy timeout to 30s to resolve 'database is locked' errors --- beets/dbcore/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 30f7ef7cf..4c68ca323 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -1100,7 +1100,7 @@ class Database: data is written in a transaction. """ - def __init__(self, path, timeout: float = 5.0): + def __init__(self, path, timeout: float = 30.0): if sqlite3.threadsafety == 0: raise RuntimeError( "sqlite3 must be compiled with multi-threading support" From 3fe085da3c86477bcbe3cc037cb09f2ff97a6def Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Fri, 6 Mar 2026 00:58:05 +0530 Subject: [PATCH 11/14] Fix mypy type error: use bytes literal for db_dir fallback dbpath is bytes (from util.bytestring_path), so os.path.abspath and os.path.dirname return bytes. The fallback value must also be bytes to satisfy mypy's type checker. Made-with: Cursor --- beets/ui/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 0860931c5..e9ce79eb3 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1520,7 +1520,7 @@ def _open_library(config: confuse.LazyConfig) -> library.Library: db_dir = os.path.dirname(normalized_path) # Handle edge case where path has no directory component if not db_dir: - db_dir = "." + db_dir = b"." raise UserError( f"database file {dbpath_display} could not be opened. " f"This may be due to a permissions issue. If the database " From 394b388b2288792f9a3f34aac0f863ab18bd5113 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 22 Mar 2026 10:14:52 +0530 Subject: [PATCH 12/14] chore: drop stale ruff ignore for renamed test_field_diff path Made-with: Cursor --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 794374b0b..7f9253476 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -329,7 +329,6 @@ ignore = [ "beets/**" = ["PT"] "test/plugins/test_ftintitle.py" = ["E501"] "test/test_util.py" = ["E501"] -"test/ui/test_field_diff.py" = ["E501"] "test/ui/test_ui_init.py" = ["PT"] "test/util/test_diff.py" = ["E501"] "test/util/test_id_extractors.py" = ["E501"] From f240fd91d060f7e121403db620b6361a04b5b880 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 22 Mar 2026 11:13:38 +0530 Subject: [PATCH 13/14] docs: move database error changelog entry under Unreleased Made-with: Cursor --- docs/changelog.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6a3e86a3d..6a37d137a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -42,6 +42,12 @@ Bug fixes - :ref:`import-cmd` Autotagging by explicit release or recording IDs now keeps candidates from all enabled metadata sources instead of dropping matches when different providers share the same ID. :bug:`6178` :bug:`6181` +- Improved error message when database cannot be opened. When SQLite fails to + open the database with 'unable to open' error, beets now provides a helpful + message suggesting it may be a permissions issue and recommends checking that + the file or directory is writable. The original SQLite error is included for + debugging. Also fixed typo in error message ('cannot not' to 'could not'). + :bug:`1676` - :doc:`plugins/mbsync` and :doc:`plugins/missing` now use each item's stored ``data_source`` for ID lookups, with a fallback to ``MusicBrainz``. - :doc:`plugins/musicbrainz`: Use ``va_name`` config for ``albumartist_sort``, @@ -286,12 +292,6 @@ New features Bug fixes ~~~~~~~~~ -- Improved error message when database cannot be opened. When SQLite fails to - open the database with 'unable to open' error, beets now provides a helpful - message suggesting it may be a permissions issue and recommends checking that - the file or directory is writable. The original SQLite error is included for - debugging. Also fixed typo in error message ('cannot not' to 'could not'). - :bug:`1676` - :doc:`/plugins/lastgenre`: Canonicalize genres when ``force`` and ``keep_existing`` are ``on``, yet no genre info on lastfm could be found. :bug:`6303` From 1f37d6c875fafa89f208ecbdaecef4585ff6c1d0 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 23 Mar 2026 11:06:04 +0530 Subject: [PATCH 14/14] docs: remove duplicate 2.6.0 changelog line (fixes Check docs CI) Made-with: Cursor --- docs/changelog.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6a37d137a..dc515e785 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -297,8 +297,6 @@ Bug fixes :bug:`6303` - Handle potential OSError when unlinking temporary files in ArtResizer. :bug:`5615` -- Handle potential OSError when unlinking temporary files in ArtResizer. - :bug:`5615` - :doc:`/plugins/spotify`: Updated Spotify API credentials. :bug:`6270` - :doc:`/plugins/smartplaylist`: Fixed an issue where multiple queries in a playlist configuration were not preserving their order, causing items to