Refactor 'osnx ls' to catch connection issues

commands/ls.sh

@@ -31,36 +31,50 @@ osnxls() {
         return 0 ;;
     esac
 
-    errors=0
+    # Why do we count successes instead of failures? Successes are easier to
+    # track down, only need to be incremented in a single code path, and can
+    # be used to determine the number of failures.
+    successes=0
 
     if [ "$#" -eq 0 ]; then
-        # like ls we will simply list the current directory
-        set -- .
+        # Like ls we will simply list the current directory. We use an empty
+        # string here for clarity when printing to the user, since they didn't
+        # pass in an argument, while we use a '.' internally for accuracy since
+        # we append a trailing slash to the path.
+        set -- ""
     fi
 
     for arg in "$@"; do
         # remove trailing slashes so we can treat these as files if we need to
         path="$(trim trailing / "$arg")"
 
-        if osnxcurl "$path/" --list-only ; then
-            continue
-        fi
+        # This function call will list both the contents of a directory, and
+        # individual files, the same behavior as ls, eliminating the need for
+        # additional code paths. This is a combination of the behavior of
+        # curl's --ftp-method=nocwd and --list-only flags, and including the
+        # trailing slash in the path.
+        #
+        # The --ftp-method=nocwd flag in osnxcurl prevents curl from changing
+        # the working directory to the final element of the directory tree
+        # before performing operations on the given path. Since we append a
+        # trailing slash to our path this will always be our directory name or,
+        # potentially, filename. Attempting to change directory to a file would
+        # cause a "(9) Server denied you to change to the given directory"
+        # error. --ftp-method=nocwd avoids this issue entirely by simply
+        # performing all operations within the FTP server's default working
+        # directory, with no directory changes.
+        if osnxcurl "${path:-.}/" --list-only ; then
+            (( successes=successes+1 ))
 
-        # error count is the number of non-directory paths we encounter
-        ((errors=errors+1))
+        elif curlexitfatal "$?" ; then
+            stderrf '%s: Failed to list path: "%s"\n' "$0" "$arg"
 
-        # removing the trailing slash causes curl to treat our path as a file
-        # curl will attempt to RETR which will only succeed if our path is a file
-        # we only retrieve the first byte to avoid downloading any more than necessary
-        if osnxcurl "$path" -r 0-0 >/dev/null; then
-            # simply print out the original argument like ls
-            echo "$arg"
-            continue
+        else
+            # If we can't list the path, and curl didn't fail to connect, then
+            # the path must not exist.
+            stderrf '%s: No such file or directory: "%s"\n' "$0" "$arg"
         fi
-
-        # if the path can't be listed or read it must not exist
-        stderrf '%s: No such file or directory\n' "$arg"
     done
 
-    return "$errors"
+    return "$(( $#-$successes ))"
 }

helpers.sh

@@ -5,6 +5,23 @@ binexists() {
     command -v "$1" >/dev/null
 }
 
+curlexitfatal() {
+    # An exit code will return 0 to signify that it's fatal, and a 0 otherwise.
+    # For documentation on curl exit codes see:
+    #   https://curl.haxx.se/libcurl/c/libcurl-errors.html
+    case "$1" in
+        19)
+            # failed to RETR path, either a directory or nonexistent
+            return 1 ;;
+        28)
+            # connection timed out, likely the server isn't reachable
+            return 0 ;;
+        *)
+            stderr "Uncaught curl exit code: $1"
+            return 0 ;;
+    esac
+}
+
 ipfrommac() {
     # we don't want a missing match to stop execution of the entire script
     # leave that logic to the main functions since this will likely be retried

osnx.sh

@@ -60,8 +60,7 @@ osnxcurl() {
         --ftp-method nocwd   \
         --user "$user:$pass"  \
         "${@:2}" -- "ftp://$ip:$port/$1"
-
-    # TODO add error handling for connection timeout
+    return "$?"
 }
 
 # shellcheck source=commands/cat.sh