Refactor errors

adit/core/errors.py

@@ -1,16 +1,16 @@
-from datetime import timedelta
-
 from rest_framework.exceptions import ErrorDetail
 
 
-class RetriableError(Exception):
-    def __init__(self, message: str, long_delay: bool = False) -> None:
-        if long_delay:
-            self.delay = timedelta(hours=24)
-        else:
-            self.delay = timedelta(minutes=15)
+class DicomConnectionError(Exception):
+    pass
+
 
-        super().__init__(message)
+class DicomCommunicationError(Exception):
+    pass
+
+
+class OutOfDiskSpaceError(Exception):
+    pass
 
 
 class BatchFileSizeError(Exception):

adit/core/tasks.py

@@ -18,7 +18,7 @@
 
 from adit.accounts.models import User
 
-from .errors import RetriableError
+from .errors import DicomCommunicationError, DicomConnectionError, OutOfDiskSpaceError
 from .models import AppSettings, DicomFolder, DicomJob, DicomTask
 from .utils.mail import (
     send_job_finished_mail,
@@ -115,15 +115,20 @@ def run(self, dicom_task_id: int):
             # When the task is rescheduled a Retry will be raised that must be
             # passed through to Celery.
             raise err
-        except RetriableError as err:
+        except (DicomCommunicationError, DicomConnectionError, OutOfDiskSpaceError) as err:
             # Inside the handle_dicom_task errors of kind RetriableTaskError can be raised
             # which are handled here and also raise a Retry in the end.
             logger.exception("Retriable error occurred during %s.", dicom_task)
 
             # We can't use the Celery built-in max_retries and celery_task.request.retries
             # directly as we also use celery_task.retry() for scheduling tasks.
             if dicom_task.retries < settings.DICOM_TASK_RETRIES:
-                logger.info("Retrying task in %s.", humanize.naturaldelta(err.delay))
+                delta = (
+                    timedelta(hours=24)
+                    if isinstance(err, OutOfDiskSpaceError)
+                    else timedelta(minutes=15)
+                )
+                logger.info("Retrying task in %s.", humanize.naturaldelta(delta))
 
                 dicom_task.status = DicomTask.Status.PENDING
                 dicom_task.message = "Task timed out and will be retried."
@@ -140,7 +145,7 @@ def run(self, dicom_task_id: int):
                 if priority < settings.CELERY_TASK_QUEUE_MAX_PRIORITY:
                     priority += 1
 
-                raise self.retry(eta=timezone.now() + err.delay, exc=err, priority=priority)
+                raise self.retry(eta=timezone.now() + delta, exc=err, priority=priority)
 
             logger.error("No more retries for finally failed %s: %s", dicom_task, str(err))
 

adit/core/utils/dicom_operator.py

@@ -23,7 +23,7 @@
 from pydicom import Dataset
 from pynetdicom.events import Event
 
-from ..errors import RetriableError
+from ..errors import DicomCommunicationError, OutOfDiskSpaceError
 from ..models import DicomServer
 from .dicom_dataset import QueryDataset, ResultDataset
 from .dicom_utils import (
@@ -461,8 +461,8 @@ def move_study(
 
         if series_list and has_failure:
             if not has_success:
-                raise RetriableError("Failed to move all series.")
-            raise RetriableError("Failed to move some series.")
+                raise DicomCommunicationError("Failed to move all series.")
+            raise DicomCommunicationError("Failed to move some series.")
 
     def move_series(self, patient_id: str, study_uid: str, series_uid: str, dest_aet: str):
         self.dimse_connector.send_c_move(
@@ -587,7 +587,7 @@ def eval_images_received():
                     if remaining_image_uids == image_uids:
                         logger.error("No images of series %s received.", series_uid)
                         receiving_errors.append(
-                            RetriableError("Failed to download all images with C-MOVE.")
+                            DicomCommunicationError("Failed to download all images with C-MOVE.")
                         )
 
                     logger.error(
@@ -596,7 +596,7 @@ def eval_images_received():
                         ", ".join(remaining_image_uids),
                     )
                     receiving_errors.append(
-                        RetriableError("Failed to download some images with C-MOVE.")
+                        DicomCommunicationError("Failed to download some images with C-MOVE.")
                     )
 
             async def handle_received_file(filename: str, metadata: Metadata):
@@ -678,8 +678,8 @@ def _handle_downloaded_image(
             if isinstance(err, OSError) and err.errno == errno.ENOSPC:
                 # No space left on destination
                 logger.exception("Out of disk space while saving %s.", file_path)
-                no_space_error = RetriableError(
-                    "Out of disk space on destination.", long_delay=True
+                no_space_error = OutOfDiskSpaceError(
+                    f"Out of disk space on destination '{dest_folder}'."
                 )
                 no_space_error.__cause__ = err
                 raise no_space_error

adit/core/utils/dicom_web_connector.py

@@ -9,7 +9,7 @@
 from pydicom.errors import InvalidDicomError
 from requests import HTTPError
 
-from ..errors import RetriableError
+from ..errors import DicomCommunicationError, DicomConnectionError
 from ..models import DicomServer
 from ..utils.dicom_dataset import QueryDataset, ResultDataset
 from ..utils.dicom_utils import read_dataset
@@ -214,7 +214,7 @@ def _send_dataset(ds: Dataset) -> None:
 
         if retriable_failures:
             plural = len(retriable_failures) > 1
-            raise RetriableError(
+            raise DicomCommunicationError(
                 f"{len(retriable_failures)} STOW-RS operation{'s' if plural else ''} failed."
             )
 
@@ -228,9 +228,9 @@ def _handle_dicomweb_error(err: HTTPError, op: str) -> NoReturn:
     status_code = err.response.status_code
     status_phrase = HTTPStatus(status_code).phrase
     if _is_retriable_error(status_code):
-        msg = f"DICOMweb {op} request failed: {status_phrase} [{status_code}]."
-        logger.error(msg)
-        raise RetriableError(msg)
+        raise DicomConnectionError(
+            f"DICOMweb {op} request failed: {status_phrase} [{status_code}]."
+        )
     else:
         logger.exception(
             f"DICOMweb {op} request failed critically: {status_phrase} [{status_code}]."

adit/core/utils/dimse_connector.py

@@ -33,7 +33,7 @@
 )
 from pynetdicom.status import code_to_category
 
-from ..errors import RetriableError
+from ..errors import DicomCommunicationError, DicomConnectionError
 from ..models import DicomServer
 from ..utils.dicom_dataset import QueryDataset, ResultDataset
 from ..utils.dicom_utils import has_wildcards, read_dataset
@@ -197,8 +197,7 @@ def _associate(self, service: DimseService):
         )
 
         if not self.assoc.is_established:
-            logger.error("Could not connect to %s.", self.server)
-            raise RetriableError(f"Could not connect to {self.server}.")
+            raise DicomConnectionError(f"Could not connect to {self.server}.")
 
     def close_connection(self):
         if not self.assoc:
@@ -245,9 +244,9 @@ def send_c_find(
         result_counter = 0
         for status, identifier in responses:
             if not status:
-                msg = "Connection timed out, was aborted or received invalid response."
-                logger.error(msg)
-                raise RetriableError(msg)
+                raise DicomConnectionError(
+                    "Connection timed out, was aborted or received invalid response."
+                )
 
             status_category = code_to_category(status.Status)
             if status_category == STATUS_SUCCESS:
@@ -266,9 +265,9 @@ def send_c_find(
                     break
 
             else:
-                msg = f"Unexpected error during C-FIND: {status_category} [{status.Status}]."
-                logger.error(msg)
-                raise RetriableError(msg)
+                raise DicomCommunicationError(
+                    f"Unexpected error during C-FIND: {status_category} [{status.Status}]."
+                )
 
     @connect_to_server("C_GET")
     def send_c_get(
@@ -350,9 +349,9 @@ def _send_dataset(ds: Dataset) -> None:
             status = self.assoc.send_c_store(ds, msg_id)
 
             if not status:
-                msg = "Connection timed out, was aborted or received invalid response."
-                logger.error(msg)
-                raise RetriableError(msg)
+                raise DicomConnectionError(
+                    "Connection timed out, was aborted or received invalid response."
+                )
             else:
                 status_category = code_to_category(status.Status)
                 if status_category == STATUS_WARNING:
@@ -409,7 +408,7 @@ def _send_dataset(ds: Dataset) -> None:
 
         if failures:
             plural = len(failures) > 1
-            raise RetriableError(
+            raise DicomCommunicationError(
                 f"{len(failures)} C-STORE operation{'s' if plural else ''} failed."
             )
 
@@ -418,9 +417,9 @@ def _handle_get_and_move_responses(
     ) -> None:
         for status, identifier in responses:
             if not status:
-                msg = "Connection timed out, was aborted or received invalid response."
-                logger.error(msg)
-                raise RetriableError(msg)
+                raise DicomConnectionError(
+                    "Connection timed out, was aborted or received invalid response."
+                )
 
             status_category = code_to_category(status.Status)
             if status_category not in [STATUS_SUCCESS, STATUS_PENDING]:
@@ -429,13 +428,13 @@ def _handle_get_and_move_responses(
                     if failed_image_uids:
                         if isinstance(failed_image_uids, str):
                             failed_image_uids = [failed_image_uids]
-                        msg = (
-                            f"Failed to transfer several images with {op}: "
-                            f"{status} [{status.Status}]."
-                        )
-                        logger.error(msg)
                         logger.error(
                             "Erroneous images (SOPInstanceUID): %s", ", ".join(failed_image_uids)
                         )
-                        raise RetriableError(msg)
-                raise RetriableError(f"Unexpected error during {op} [status {status_category}].")
+                        raise DicomCommunicationError(
+                            f"Failed to transfer several images with {op}: "
+                            f"{status} [{status.Status}]."
+                        )
+                raise DicomCommunicationError(
+                    f"Unexpected error during {op} [status {status_category}]."
+                )