Skip to content

Commit

Permalink
Process manager should not attempt to unlink the processes database f…
Browse files Browse the repository at this point in the history
…ile in case of OperationalError
  • Loading branch information
kozlovsky committed Mar 18, 2024
1 parent b64ec7b commit f34d807
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
19 changes: 13 additions & 6 deletions src/tribler/core/utilities/process_manager/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,21 @@ def connect(self) -> ContextManager[sqlite3.Connection]:
if connection:
connection.close()

if isinstance(e, sqlite3.DatabaseError):
self.db_filepath.unlink(missing_ok=True)
if isinstance(e, sqlite3.OperationalError):

if str(e) == 'unable to open database file':
msg = f"{e}: {self._unable_to_open_db_file_get_reason()}"
raise sqlite3.OperationalError(msg) from e

if isinstance(e, sqlite3.OperationalError) and str(e) == 'unable to open database file':
msg = f"{e}: {self._unable_to_open_db_file_get_reason()}"
raise sqlite3.OperationalError(msg) from e
raise e

if isinstance(e, sqlite3.DatabaseError):
try:
self.db_filepath.unlink(missing_ok=True)
except PermissionError:
logger.warning(f'Unable to delete the processes database file: {self.db_filepath}')

raise
raise e

def _unable_to_open_db_file_get_reason(self):
dir_path = self.db_filepath.parent
Expand Down
30 changes: 30 additions & 0 deletions src/tribler/core/utilities/process_manager/tests/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,33 @@ def test_unable_to_open_db_file_reason_added(process_manager):
match=r'^unable to open database file: parent directory.*does not exist$'):
with process_manager.connect():
pass


@patch('sqlite3.connect', MagicMock(side_effect=sqlite3.OperationalError('error text')))
def test_connect_operational_error(process_manager):
# All OperationalError exceptions except "unable to open database file" should be re-raised
with pytest.raises(sqlite3.OperationalError, match=r'^error text$'):
with process_manager.connect():
pass

Check warning on line 292 in src/tribler/core/utilities/process_manager/tests/test_manager.py

View check run for this annotation

Codecov / codecov/patch

src/tribler/core/utilities/process_manager/tests/test_manager.py#L292

Added line #L292 was not covered by tests


@patch('sqlite3.connect', MagicMock(side_effect=sqlite3.DatabaseError('error text')))
@patch('pathlib.Path.unlink', MagicMock(side_effect=PermissionError))
def test_connect_database_error_suppress_permission_error(process_manager):
# All OperationalError exceptions except "unable to open database file" should be re-raised
with pytest.raises(sqlite3.DatabaseError, match=r'^error text$'):
with process_manager.connect():
pass

Check warning on line 301 in src/tribler/core/utilities/process_manager/tests/test_manager.py

View check run for this annotation

Codecov / codecov/patch

src/tribler/core/utilities/process_manager/tests/test_manager.py#L301

Added line #L301 was not covered by tests


class TestError(Exception):
pass


@patch('sqlite3.connect', MagicMock(side_effect=sqlite3.DatabaseError('error text')))
@patch('pathlib.Path.unlink', MagicMock(side_effect=TestError))
def test_connect_database_error_raise_non_permission_error(process_manager):
# All OperationalError exceptions except "unable to open database file" should be re-raised
with pytest.raises(TestError):
with process_manager.connect():
pass

Check warning on line 314 in src/tribler/core/utilities/process_manager/tests/test_manager.py

View check run for this annotation

Codecov / codecov/patch

src/tribler/core/utilities/process_manager/tests/test_manager.py#L314

Added line #L314 was not covered by tests

0 comments on commit f34d807

Please # to comment.