From 955dfea866550cfdf70576aefdff33026d499f30 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Fri, 29 May 2020 16:58:30 -0700 Subject: [PATCH] storage: Fix regression with showing error messages In ed09028fcd2c850c3b87b65de66187b214190150, when eject was made to run as superuser inside storage action, parsing of the error messages was not handled properly. Fix it to show simple error messages about why the eject was not successful. Tests performed: - In a terminal, switch to the directory where a disk is mounted to keep the mount point busy. Attempt to eject the disk. A large stack trace is shown without the patch and a clean error message is shown with it. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- actions/storage | 8 ++- plinth/modules/storage/__init__.py | 79 +++++++++++++++--------------- plinth/modules/storage/views.py | 10 ++-- 3 files changed, 50 insertions(+), 47 deletions(-) diff --git a/actions/storage b/actions/storage index 6d5b97602..44da86c53 100755 --- a/actions/storage +++ b/actions/storage @@ -259,8 +259,12 @@ def subcommand_mount(arguments): def subcommand_eject(arguments): """Eject a device by its path.""" device_path = arguments.device - drive = eject_drive_of_device(device_path) - print(json.dumps(drive)) + try: + drive = eject_drive_of_device(device_path) + print(json.dumps(drive)) + except Exception as exception: + print(exception, file=sys.stderr) + sys.exit(1) def _get_options(): diff --git a/plinth/modules/storage/__init__.py b/plinth/modules/storage/__init__.py index 313089e89..b2d0e1949 100644 --- a/plinth/modules/storage/__init__.py +++ b/plinth/modules/storage/__init__.py @@ -15,7 +15,7 @@ from plinth import actions from plinth import app as app_module from plinth import cfg, glib, menu from plinth.errors import ActionError, PlinthError -from plinth.utils import format_lazy, import_from_gi +from plinth.utils import format_lazy from . import udisks2 from .manifest import backup # noqa, pylint: disable=unused-import @@ -198,45 +198,46 @@ def format_bytes(size): def get_error_message(error): """Return an error message given an exception.""" - udisks = import_from_gi('UDisks', '2.0') - if error.matches(udisks.Error.quark(), udisks.Error.FAILED): - message = _('The operation failed.') - elif error.matches(udisks.Error.quark(), udisks.Error.CANCELLED): - message = _('The operation was cancelled.') - elif error.matches(udisks.Error.quark(), udisks.Error.ALREADY_UNMOUNTING): - message = _('The device is already unmounting.') - elif error.matches(udisks.Error.quark(), udisks.Error.NOT_SUPPORTED): - message = _('The operation is not supported due to ' - 'missing driver/tool support.') - elif error.matches(udisks.Error.quark(), udisks.Error.TIMED_OUT): - message = _('The operation timed out.') - elif error.matches(udisks.Error.quark(), udisks.Error.WOULD_WAKEUP): - message = _('The operation would wake up a disk that is ' - 'in a deep-sleep state.') - elif error.matches(udisks.Error.quark(), udisks.Error.DEVICE_BUSY): - message = _('Attempting to unmount a device that is busy.') - elif error.matches(udisks.Error.quark(), udisks.Error.ALREADY_CANCELLED): - message = _('The operation has already been cancelled.') - elif error.matches(udisks.Error.quark(), udisks.Error.NOT_AUTHORIZED) or \ - error.matches(udisks.Error.quark(), - udisks.Error.NOT_AUTHORIZED_CAN_OBTAIN) or \ - error.matches(udisks.Error.quark(), - udisks.Error.NOT_AUTHORIZED_DISMISSED): - message = _('Not authorized to perform the requested operation.') - elif error.matches(udisks.Error.quark(), udisks.Error.ALREADY_MOUNTED): - message = _('The device is already mounted.') - elif error.matches(udisks.Error.quark(), udisks.Error.NOT_MOUNTED): - message = _('The device is not mounted.') - elif error.matches(udisks.Error.quark(), - udisks.Error.OPTION_NOT_PERMITTED): - message = _('Not permitted to use the requested option.') - elif error.matches(udisks.Error.quark(), - udisks.Error.MOUNTED_BY_OTHER_USER): - message = _('The device is mounted by another user.') - else: - message = error.message + error_parts = error.split(':') + if error_parts[0] != 'udisks-error-quark': + return error - return message + short_error = error_parts[2].strip().split('.')[-1] + message_map = { + 'Failed': + _('The operation failed.'), + 'Cancelled': + _('The operation was cancelled.'), + 'AlreadyUnmounting': + _('The device is already unmounting.'), + 'NotSupported': + _('The operation is not supported due to missing driver/tool ' + 'support.'), + 'TimedOut': + _('The operation timed out.'), + 'WouldWakeup': + _('The operation would wake up a disk that is in a deep-sleep ' + 'state.'), + 'DeviceBusy': + _('Attempting to unmount a device that is busy.'), + 'AlreadyCancelled': + _('The operation has already been cancelled.'), + 'NotAuthorized': + _('Not authorized to perform the requested operation.'), + 'NotAuthorizedCanObtain': + _('Not authorized to perform the requested operation.'), + 'NotAuthorizedDismissed': + _('Not authorized to perform the requested operation.'), + 'AlreadyMounted': + _('The device is already mounted.'), + 'NotMounted': + _('The device is not mounted.'), + 'OptionNotPermitted': + _('Not permitted to use the requested option.'), + 'MountedByOtherUser': + _('The device is mounted by another user.') + } + return message_map.get(short_error, error) def setup(helper, old_version=None): diff --git a/plinth/modules/storage/views.py b/plinth/modules/storage/views.py index 7a239372f..d1294ca78 100644 --- a/plinth/modules/storage/views.py +++ b/plinth/modules/storage/views.py @@ -15,6 +15,7 @@ from django.utils.translation import ugettext as _ from django.views.decorators.http import require_POST from plinth import actions, views +from plinth.errors import ActionError from plinth.modules import storage from . import get_error_message @@ -92,13 +93,10 @@ def eject(request, device_path): drive_model=drive['model'])) else: messages.success(request, _('Device can be safely unplugged.')) - except Exception as exception: - try: - message = get_error_message(exception) - except AttributeError: - message = str(exception) + except ActionError as exception: + message = get_error_message(exception.args[2]) - logger.exception('Error ejecting device - %s', message) + logger.error('Error ejecting device - %s', message) messages.error( request, _('Error ejecting device: {error_message}').format(