Handle PermissionErrors on downlink (#150)

* Handle PermissionErrors on downlink

* spelling and formatting

* Raise PermissionError instead of warning

.github/actions/spelling/expect.txt

@@ -572,6 +572,7 @@ Smath
 SNDHWM
 socketserver
 sorttable
+spam
 sphinxcontrib
 splitext
 squashify

src/fprime_gds/common/files/downlinker.py

@@ -52,7 +52,6 @@ def __init__(self, directory, timeout=20.0, log_dir=None):
         self.sequence = 0  # Keeps track of what the current sequence ID should be
         self.timer = fprime_gds.common.files.helpers.Timeout()
         self.timer.setup(self.timeout, timeout)
-        os.makedirs(self.__directory, exist_ok=True)
 
     def data_callback(self, data, sender=None):
         """
@@ -96,7 +95,15 @@ def handle_start(self, data):
             size,
             self.__log_dir,
         )
-        self.active.open(TransmitFileState.WRITE)
+        try:
+            self.active.open(TransmitFileState.WRITE)
+        except PermissionError as exc:
+            self.state = FileStates.ERROR
+            LOGGER.warning(
+                "Unable to open file for writing. Discarding subsequent downlink packets. "
+                + str(exc)
+            )
+            return
         LOGGER.addHandler(self.active.log_handler)
         message = "Received START packet with metadata:\n"
         message += "\tSize: %d\n"
@@ -116,7 +123,10 @@ def handle_data(self, data):
         # Initialize all relevant DATA packet attributes into variables from file_data
         offset = data.offset
         if self.state != FileStates.RUNNING:
-            LOGGER.warning("Received unexpected data packet for offset: %d", offset)
+            # ERROR state means GDS is not ready to receive data packets (permission error)
+            # No need to log this, as it is already logged in handle_start and would only spam logs
+            if self.state != FileStates.ERROR:
+                LOGGER.warning("Received unexpected data packet for offset: %d", offset)
         else:
             if data.seqID != self.sequence:
                 LOGGER.warning(
@@ -156,9 +166,10 @@ def handle_end(self, data):
         """
         # Initialize all relevant END packet attributes into variables from file_data
         # hashValue attribute is not relevant right now, but might be in the future
-        if self.state != FileStates.RUNNING:
-            LOGGER.warning("Received unexpected END packet")
-        else:
+        if self.state == FileStates.ERROR:
+            LOGGER.info("Received END packet for discarded downlink")
+            self.finish()
+        elif self.state == FileStates.RUNNING:
             if data.seqID != self.sequence:
                 LOGGER.warning(
                     "End packet has unexpected sequence id. Expected: %d got %d",
@@ -167,6 +178,8 @@ def handle_end(self, data):
                 )
             LOGGER.info("Received END packet, finishing downlink")
             self.finish()
+        else:
+            LOGGER.warning("Received unexpected END packet")
 
     def timeout(self):
         """Timeout the current downlink"""

src/fprime_gds/common/files/helpers.py

@@ -77,6 +77,7 @@ class FileStates(enum.Enum):
     RUNNING = 1
     CANCELED = 2
     END_WAIT = 3  # Waiting for the handshake for CANCEL or END packet
+    ERROR = 4
 
 
 class CFDPChecksum:

src/fprime_gds/common/pipeline/files.py

@@ -7,6 +7,7 @@
 
 @author mstarch
 """
+import os
 import fprime_gds.common.files.downlinker
 import fprime_gds.common.files.uplinker
 
@@ -27,7 +28,8 @@ def setup_file_handling(
         self, down_store, file_encoder, file_decoder, distributor, log_dir
     ):
         """
-        Sets up the file handling (uplink and downlink) from a pair of encoders and decoders
+        Sets up the file handling (uplink and downlink) from a pair of encoders and decoders.
+        Raises a PermissionError if the down_store is not writable.
 
         :param down_store: downlink storage directory
         :param file_encoder: file encoder for uplink
@@ -41,6 +43,11 @@ def setup_file_handling(
         )
         file_decoder.register(self.__downlinker)
         distributor.register("FW_PACKET_HAND", self.__uplinker)
+        if not os.access(down_store, os.W_OK):
+            raise PermissionError(
+                f"{down_store} is not writable. Downlinker not be able to save files. "
+                "Fix permissions or change storage directory with --file-storage-directory."
+            )
 
     @property
     def uplinker(self):