Better error reporting.

- Use ex-info instead of Exception. Return plenty of metadata with each
  exception. Most deliberate exceptions now will contain a reference to
  the clj-libssh2.session.Session being used when it was thrown.
- Improve some error messages to be more accurate and/or less confusing.

Fixes #5

Author: conormcd

src/clj_libssh2/agent.clj

@@ -2,7 +2,8 @@
   "Functions for interacting with an SSH agent. The agent is expected to be
    available on the UNIX domain socket referred to by the SSH_AUTH_SOCK
    environment variable."
-  (:require [clj-libssh2.error :refer [handle-errors with-timeout]]
+  (:require [clj-libssh2.error :as error :refer [handle-errors with-timeout]]
+            [clj-libssh2.libssh2 :as libssh2]
             [clj-libssh2.libssh2.agent :as libssh2-agent])
   (:import [com.sun.jna.ptr PointerByReference]))
 
@@ -32,7 +33,10 @@
     (case ret
       0 (.getValue id)
       1 nil
-      (throw (Exception. "An unknown error occurred")))))
+      (throw (ex-info "libssh2_agent_get_identity returned a bad value."
+                      {:function "libssh2_agent_get_identity"
+                       :return ret
+                       :session session})))))
 
 (defn authenticate
   "Attempt to authenticate a session using the agent.
@@ -50,7 +54,7 @@
   [session username]
   (let [ssh-agent (libssh2-agent/init (:session session))]
     (when (nil? ssh-agent)
-      (throw (Exception. "Failed to initialize agent.")))
+      (error/maybe-throw-error session libssh2/ERROR_ALLOC))
     (try
       (handle-errors session
         (with-timeout :agent
@@ -65,7 +69,9 @@
                                (libssh2-agent/userauth ssh-agent username id)))
                         id)
                       false)))
-        (throw (Exception. "Failed to authenticate with the agent.")))
+        (throw (ex-info "Failed to authenticate using the SSH agent."
+                        {:username username
+                         :session session})))
       true
       (finally
         (handle-errors session

src/clj_libssh2/authentication.clj

@@ -4,7 +4,8 @@
             [clj-libssh2.agent :as agent]
             [clj-libssh2.error :refer [handle-errors with-timeout]]
             [clj-libssh2.libssh2.userauth :as libssh2-userauth])
-  (:import clojure.lang.PersistentArrayMap))
+  (:import [java.io FileNotFoundException]
+           [clojure.lang PersistentArrayMap]))
 
 (defprotocol Credentials
   "A datatype to represent a way of authentication and the necessary data to
@@ -21,9 +22,7 @@
   (valid? [credentials] (and (some? username)
                              (some? passphrase)
                              (some? private-key)
-                             (some? public-key)
-                             (.exists (file private-key))
-                             (.exists (file public-key)))))
+                             (some? public-key))))
 
 (defrecord PasswordCredentials [username password]
   Credentials
@@ -54,7 +53,7 @@
   [session credentials]
   (doseq [keyfile (map #(% credentials) [:private-key :public-key])]
     (when-not (.exists (file keyfile))
-      (throw (Exception. (format "%s does not exist." keyfile)))))
+      (throw (FileNotFoundException. keyfile))))
   (handle-errors session
     (with-timeout :auth
       (libssh2-userauth/publickey-fromfile (:session session)
@@ -81,4 +80,6 @@
         (authenticate session creds)
         (if (< 1 (count m))
           (recur (rest m))
-          (throw (Exception. "Invalid credentials")))))))
+          (throw (ex-info "Failed to determine credentials type."
+                          {:items (keys credentials)
+                           :session session})))))))

src/clj_libssh2/channel.clj

@@ -164,7 +164,8 @@
                       :else (str v)))
         fail-if-forbidden (fn [ret]
                             (if (= libssh2/ERROR_CHANNEL_REQUEST_DENIED ret)
-                              (throw (Exception. "Setting environment variables is not permitted."))
+                              (throw (ex-info "Setting environment variables is not permitted."
+                                              {:session session}))
                               ret))]
     (doseq [[k v] env]
       (block session
@@ -322,9 +323,11 @@
       (when (and (= pump-fn pull)
                  (= :eagain new-status)
                  (< (-> session :options :read-timeout) (- now last-read-time)))
-        (throw (Exception. (format "Read timeout for %s stream %d"
-                                   (-> stream :direction name)
-                                   (-> stream :id)))))
+        (throw (ex-info "Read timeout on a channel."
+                        {:direction (-> stream :direction name)
+                         :id (-> stream :id)
+                         :timeout (-> session :options :read-timeout)
+                         :session session})))
       (assoc stream :status new-status :last-read-time now))
     stream))
 

src/clj_libssh2/error.clj

@@ -109,8 +109,9 @@
    which may be useful to debug the error handling itself:
 
    :error         The error message from error-messages, if any.
-   :session-error The error message from session-error-message, if any.
-   :error-code    The numeric return value."
+   :error-code    The numeric return value.
+   :session       The clj-libssh2.session.Session object, if any.
+   :session-error The error message from session-error-message, if any."
   [session error-code]
   (when (and (some? error-code)
              (> 0 error-code)
@@ -121,8 +122,9 @@
                           default-message
                           (format "An unknown error occurred: %d" error-code))
                       {:error default-message
-                       :session-error session-message
-                       :error-code error-code})))))
+                       :error-code error-code
+                       :session session
+                       :session-error session-message})))))
 
 (defmacro handle-errors
   "Run some code that might return a negative number to indicate an error.
@@ -197,7 +199,9 @@
          timeout# (get-timeout ~timeout)]
      (loop [timedout# false]
        (if timedout#
-         (throw (Exception. "Timeout!"))
+         (throw (ex-info "Timeout exceeded."
+                         {:timeout ~timeout
+                          :timeout-length timeout#}))
          (let [r# (do ~@body)]
            (if (= r# libssh2/ERROR_EAGAIN)
              (recur (< timeout# (- (System/currentTimeMillis) start#)))

src/clj_libssh2/known_hosts.clj

@@ -2,7 +2,7 @@
   "Utilities for checking the fingerprint of a remote machine against a list of
    known hosts."
   (:require [clojure.java.io :refer [file]]
-            [clj-libssh2.error :refer [handle-errors with-timeout]]
+            [clj-libssh2.error :as error :refer [handle-errors with-timeout]]
             [clj-libssh2.libssh2 :as libssh2]
             [clj-libssh2.libssh2.knownhost :as libssh2-knownhost]
             [clj-libssh2.libssh2.session :as libssh2-session])
@@ -39,7 +39,9 @@
                                         libssh2/ERROR_HOSTKEY_SIGN
                                         0)
     libssh2/KNOWNHOST_CHECK_FAILURE   libssh2/ERROR_HOSTKEY_SIGN
-    (throw (Exception. (format "Unknown return code from libssh2-knownhost/checkp: %d" result)))))
+    (throw (ex-info "Unknown return code from libssh2_knownhost_checkp."
+                    {:function "libssh2_knownhost_checkp"
+                     :return result}))))
 
 (defn- host-fingerprint
   "Get the remote host's fingerprint.
@@ -131,7 +133,7 @@
         fail-on-mismatch (-> session-options :fail-unless-known-hosts-matches)
         fail-on-missing (-> session-options :fail-if-not-in-known-hosts)]
     (when (nil? known-hosts)
-      (throw (Exception. "Failed to initialize known hosts store.")))
+      (error/maybe-throw-error session libssh2/ERROR_ALLOC))
     (try
       (load-known-hosts session known-hosts file)
       (check-fingerprint session

src/clj_libssh2/session.clj

@@ -4,7 +4,7 @@
             [clj-libssh2.libssh2 :as libssh2]
             [clj-libssh2.libssh2.session :as libssh2-session]
             [clj-libssh2.authentication :refer [authenticate]]
-            [clj-libssh2.error :refer [handle-errors with-timeout]]
+            [clj-libssh2.error :as error :refer [handle-errors with-timeout]]
             [clj-libssh2.known-hosts :as known-hosts]
             [clj-libssh2.socket :as socket]))
 
@@ -37,7 +37,7 @@
   []
   (let [session (libssh2-session/init)]
     (when-not session
-      (throw (Exception. "Failed to create a libssh2 session.")))
+      (error/maybe-throw-error nil libssh2/ERROR_ALLOC))
     session))
 
 (defn- create-session-options
@@ -73,15 +73,15 @@
    nil or throws an exception if requested."
   ([session]
    (destroy-session session "Shutting down normally." false))
-  ([{session :session} reason raise]
+  ([session reason raise]
    (handle-errors nil
     (with-timeout :session
-      (libssh2-session/disconnect session reason)))
+      (libssh2-session/disconnect (:session session) reason)))
    (handle-errors nil
     (with-timeout :session
-      (libssh2-session/free session)))
+      (libssh2-session/free (:session session))))
    (when raise
-     (throw (Exception. reason)))))
+     (throw (ex-info reason {:session session})))))
 
 (defn- handshake
   "Perform the startup handshake with the remote host.
@@ -147,9 +147,9 @@
       (authenticate session credentials)
       (swap! sessions conj session)
       session
-      (catch Exception e
+      (catch Throwable t
         (close session)
-        (throw e)))))
+        (throw t)))))
 
 (defmacro with-session
   "A convenience macro for running some code with a particular session.

src/clj_libssh2/socket.clj

@@ -29,17 +29,18 @@
                            port)]
     (when (> 0 socket)
       ;; Magic numbers are from libsimplesocket.h
-      (throw (Exception. (condp = socket
-                           SIMPLE_SOCKET_BAD_ADDRESS
-                           (format "%s is not a valid IP address" address)
+      (let [message (condp = socket
+                      SIMPLE_SOCKET_BAD_ADDRESS
+                      (format "%s is not a valid IP address" address)
 
-                           SIMPLE_SOCKET_SOCKET_FAILED
-                           "Failed to create a TCP socket"
+                      SIMPLE_SOCKET_SOCKET_FAILED
+                      "Failed to create a TCP socket"
 
-                           SIMPLE_SOCKET_CONNECT_FAILED
-                           (format "Failed to connect to %s:%d" address port)
+                      SIMPLE_SOCKET_CONNECT_FAILED
+                      (format "Failed to connect to %s:%d" address port)
 
-                           "An unknown error occurred"))))
+                      "simple_socket_connect returned a bad value")]
+        (throw (ex-info message {:socket socket}))))
     socket))
 
 (defn select

test/clj_libssh2/test_authentication.clj

@@ -16,7 +16,7 @@
     (session/close session))
   (is (= 0 (count @session/sessions))))
 
-; This is more fully tested in clj-libssh2.test-authentication
+; This is more fully tested in clj-libssh2.test-agent
 (deftest agent-authentication-works
   (is (auth (->AgentCredentials (test/ssh-user)))))
 
@@ -49,10 +49,10 @@
       (is (valid? unauthorized))
       (is (thrown? Exception (auth unauthorized))))
     (testing "It fails if the private key file doesn't exist"
-      (is (not (valid? bad-privkey)))
+      (is (valid? bad-privkey))
       (is (thrown? Exception (auth bad-privkey))))
     (testing "It fails if the public key file doesn't exist"
-      (is (not (valid? bad-pubkey)))
+      (is (valid? bad-pubkey))
       (is (thrown? Exception (auth bad-pubkey))))
     (testing "It fails if the passphrase is incorrect"
       (is (valid? with-wrong-passphrase))
@@ -78,5 +78,5 @@
 (deftest authenticating-with-a-map-fails-if-there's-no-equivalent-record
   (is (thrown-with-msg?
         Exception
-        #"Invalid credentials"
+        #"Failed to determine credentials type"
         (auth {:password "foo"}))))