Merge pull request #24394 from martyngigg/errorreporter_no_recovery

Fix error reporter when recovery directory does not exist

scripts/ErrorReporter/error_report_presenter.py

@@ -4,9 +4,11 @@
 #     NScD Oak Ridge National Laboratory, European Spallation Source
 #     & Institut Laue - Langevin
 # SPDX - License - Identifier: GPL - 3.0 +
+import os
+
 from mantid.kernel import ErrorReporter, UsageService, ConfigService
 from mantid.kernel import Logger
-from ErrorReporter.retrieve_recovery_files import zip_recovery_directory, remove_recovery_file
+from ErrorReporter.retrieve_recovery_files import zip_recovery_directory
 import requests
 
 
@@ -31,13 +33,21 @@ class ErrorReporterPresenter(object):
 
     def share_all_information(self, continue_working, name, email, text_box):
         uptime = UsageService.getUpTime()
-        zip_recovery_file, file_hash = zip_recovery_directory()
-        status = self._send_report_to_server(share_identifiable=True, uptime=uptime, name=name, email=email, file_hash=file_hash
-                                             , text_box=text_box)
-        self.error_log.notice("Sent complete information")
-        if status == 201:
-            self._upload_recovery_file(zip_recovery_file=zip_recovery_file)
-        remove_recovery_file(zip_recovery_file)
+        try:
+            recovery_archive, file_hash = zip_recovery_directory()
+        except Exception as exc:
+            self.error_log.information("Error creating recovery archive: {}. No recovery information will be sent")
+            recovery_archive, file_hash = None, ""
+        status = self._send_report_to_server(share_identifiable=True, uptime=uptime, name=name, email=email, file_hash=file_hash,
+                                             text_box=text_box)
+        self.error_log.notice("Sent full information")
+        if status == 201 and recovery_archive:
+            self._upload_recovery_file(recovery_archive=recovery_archive)
+            try:
+                os.remove(recovery_archive)
+            except OSError as exc:
+                self.error_log.information("Unable to remove zipped recovery information: {}".format(str(exc)))
+
         self._handle_exit(continue_working)
         return status
 
@@ -62,13 +72,13 @@ class ErrorReporterPresenter(object):
         else:
             self.error_log.error("Continue working.")
 
-    def _upload_recovery_file(self, zip_recovery_file):
+    def _upload_recovery_file(self, recovery_archive):
         url = ConfigService['errorreports.rooturl']
         url = '{}/api/recovery'.format(url)
-        files = {'file': open('{}.zip'.format(zip_recovery_file), 'rb')}
+        files = {'file': open('{}'.format(recovery_archive), 'rb')}
         response = requests.post(url, files=files)
         if response.status_code == 201:
-            self.error_log.notice("Uploaded recovery file to server HTTP response {}".format(response.status_code))
+            self.error_log.notice("Uploaded recovery file to server. HTTP response {}".format(response.status_code))
         else:
             self.error_log.error("Failed to send recovery data HTTP response {}".format(response.status_code))
 

scripts/ErrorReporter/retrieve_recovery_files.py

@@ -10,31 +10,23 @@ def get_properties_directory():
 
 
 def get_recovery_files_path():
-    recovery_files_path = ''
     properties_directory = get_properties_directory()
     if 'recovery' not in os.listdir(properties_directory):
-        return recovery_files_path
+        return None
 
-    recovery_dir_contents = os.listdir(properties_directory + 'recovery')
-    if not recovery_dir_contents:
+    recovery_files_path = os.path.join(properties_directory, 'recovery')
+    if len(os.listdir(recovery_files_path)) > 0:
         return recovery_files_path
-
-    recovery_files_path = properties_directory + 'recovery'
-    return recovery_files_path
+    else:
+        return None
 
 
 def zip_recovery_directory():
     path = get_recovery_files_path()
+    if path is None:
+        return "", ""
     directory = get_properties_directory()
     hash_value = hashlib.md5(str.encode(directory + str(datetime.datetime.now())))
-    zip_file = os.path.join(directory, hash_value.hexdigest())
-    if path:
-        shutil.make_archive(zip_file, 'zip', path)
-        return zip_file, hash_value.hexdigest()
-    return ''
-
-
-def remove_recovery_file(file):
-    directory = get_properties_directory()
-    zip_file = os.path.join(directory, file)
-    os.remove(zip_file + '.zip')
+    base_name = os.path.join(directory, hash_value.hexdigest())
+    zip_file = shutil.make_archive(base_name, 'zip', path)
+    return zip_file, hash_value.hexdigest()

scripts/test/ErrorReportPresenterTest.py

@@ -26,10 +26,6 @@ class ErrorReportPresenterTest(unittest.TestCase):
         self.zip_recovery_mock = zip_recovery_patcher.start()
         self.zip_recovery_mock.return_value = ('zipped_file', 'file_hash')
 
-        file_removal_patcher = mock.patch('ErrorReporter.error_report_presenter.remove_recovery_file')
-        self.addCleanup(file_removal_patcher.stop)
-        self.file_removal_mock = file_removal_patcher.start()
-
         self.view = mock.MagicMock()
         self.exit_code = 255
         self.error_report_presenter = ErrorReporterPresenter(self.view, self.exit_code)
@@ -97,7 +93,7 @@ class ErrorReportPresenterTest(unittest.TestCase):
         self.error_report_presenter._send_report_to_server = mock.MagicMock(return_value=201)
         self.error_report_presenter._upload_recovery_file = mock.MagicMock()
         self.error_report_presenter._handle_exit = mock.MagicMock()
-        
+
         self.error_report_presenter.error_handler(continue_working, share, name, email, text_box)
 
         self.error_report_presenter._send_report_to_server.called_once_with(share_identifiable=True, name=name, email=email,