Skip to content

Commit

Permalink
Merge pull request #7932 from kozlovsky/fix/process_manager_connect_p…
Browse files Browse the repository at this point in the history
…ermission_error

Process manager should not attempt to unlink the processes database file in case of OperationalError
  • Loading branch information
kozlovsky authored Mar 19, 2024
2 parents 0cc40fb + c87dfbc commit 288f3d7
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 # pragma: no cover


@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 # pragma: no cover


@patch('sqlite3.connect', MagicMock(side_effect=sqlite3.DatabaseError('error text')))
def test_connect_database_error_raise_non_permission_error(process_manager):

class TestError(Exception):
pass

with patch('pathlib.Path.unlink', MagicMock(side_effect=TestError)):
# All OperationalError exceptions except "unable to open database file" should be re-raised
with pytest.raises(TestError):
with process_manager.connect():
pass # pragma: no cover

0 comments on commit 288f3d7

Please sign in to comment.