From 1f43be95a096df353eaf349644c300d37e0a00c3 Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Tue, 10 Feb 2015 14:54:53 +0530
Subject: [PATCH] upgrades: Handle action errors using error codes

- This is more reliable than string parsing in the output.

- We will be using pythonic try/catches instead of if conditions.
---
 actions/upgrades                              | 27 ++++++++++++-------
 .../upgrades/templates/upgrades_run.html      |  3 ++-
 plinth/modules/upgrades/upgrades.py           | 15 ++++++-----
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/actions/upgrades b/actions/upgrades
index 05cd68c16..b6b9d547d 100755
--- a/actions/upgrades
+++ b/actions/upgrades
@@ -24,6 +24,7 @@ import argparse
 import os
 import re
 import subprocess
+import sys
 
 CONF_FILE = '/etc/apt/apt.conf.d/50unattended-upgrades'
 AUTO_CONF_FILE = '/etc/apt/apt.conf.d/20auto-upgrades'
@@ -55,24 +56,31 @@ def subcommand_run(_):
     try:
         setup()
     except FileNotFoundError:
-        print('Error: Could not configure unattended-upgrades.')
-        return
+        print('Error: Could not configure unattended-upgrades.',
+              file=sys.stderr)
+        sys.exit(1)
 
     try:
         output = subprocess.check_output(['unattended-upgrades', '-v'])
-    except subprocess.CalledProcessError as error:
-        print('Error: %s', error)
     except FileNotFoundError:
-        print('Error: unattended-upgrades is not available.')
+        print('Error: unattended-upgrades is not available.', file=sys.stderr)
+        sys.exit(2)
+    except Exception as error:
+        print('Error: {0}'.format(error), file=sys.stderr)
+        sys.exit(3)
     else:
-        print('%s', output.decode())
+        print(output.decode())
 
 
 def subcommand_check_auto(_):
     """Check if automatic upgrades are enabled"""
     arguments = ['apt-config', 'shell', 'UpdateInterval',
                  'APT::Periodic::Update-Package-Lists']
-    output = subprocess.check_output(arguments).decode()
+    try:
+        output = subprocess.check_output(arguments).decode()
+    except subprocess.CalledProcessError as error:
+        print('Error: {0}'.format(error), file=sys.stderr)
+        sys.exit(1)
 
     update_interval = 0
     match = re.match(r"UpdateInterval='(.*)'", output)
@@ -87,8 +95,9 @@ def subcommand_enable_auto(_):
     try:
         setup()
     except FileNotFoundError:
-        print('Error: Could not configure unattended-upgrades.')
-        return
+        print('Error: Could not configure unattended-upgrades.',
+              file=sys.stderr)
+        sys.exit(1)
 
     with open(AUTO_CONF_FILE, 'w') as conffile:
         conffile.write('APT::Periodic::Update-Package-Lists "1";\n')
diff --git a/plinth/modules/upgrades/templates/upgrades_run.html b/plinth/modules/upgrades/templates/upgrades_run.html
index ecfa90525..7c2c5b59b 100644
--- a/plinth/modules/upgrades/templates/upgrades_run.html
+++ b/plinth/modules/upgrades/templates/upgrades_run.html
@@ -24,8 +24,9 @@
 
 {% if upgrades_error %}
   <div class="alert alert-danger" role="alert">
-    There was an error while upgrading:
+    There was an error while upgrading.
   </div>
+  <h5>Output from unattended-upgrades:</h5>
   <pre>{{ upgrades_error }}</pre>
 {% endif %}
 
diff --git a/plinth/modules/upgrades/upgrades.py b/plinth/modules/upgrades/upgrades.py
index a3652d9a4..842d3e06f 100644
--- a/plinth/modules/upgrades/upgrades.py
+++ b/plinth/modules/upgrades/upgrades.py
@@ -124,13 +124,16 @@ def _apply_changes(request, old_status, new_status):
     else:
         option = 'disable-auto'
 
-    output = actions.superuser_run('upgrades', [option])
+    try:
+        actions.superuser_run('upgrades', [option])
+    except ActionError as exception:
+        error = exception.args[2]
+        messages.error(
+            request, _('Error when configuring unattended-upgrades: %s') %
+            error)
+        return
 
-    if 'Error' in output:
-        messages.error(request,
-                       _('Error when configuring unattended-upgrades: %s') %
-                       output)
-    elif option == 'enable-auto':
+    if option == 'enable-auto':
         messages.success(request, _('Automatic upgrades enabled'))
     else:
         messages.success(request, _('Automatic upgrades disabled'))