Display api errors as notifications

src/cljs/airsonic_ui/events.cljs

@@ -11,6 +11,12 @@
 ;; ::events/something-happening -> relevant to only this app
 ;; :single-colon/something -> coming from external sources (e.g. :audio/... or :routes/...) that are potentially reusable
 
+(re-frame/reg-fx
+ ;; a simple effect to keep println statements out of our event handlers
+ :log
+ (fn [params]
+   (apply println params)))
+
 ;; database reset / init
 
 (re-frame/reg-event-db
@@ -29,12 +35,24 @@
    :http-xhrio {:method :get
                 :uri (api/url server "ping" {:u user :p pass})
                 :response-format (ajax/json-response-format {:keywords? true})
-                :on-success [::credentials-verified user pass]
-                :on-failure [::api-failure]}})
+                :on-success [::verify-auth-response user pass]
+                :on-failure [:api/bad-response]}})
 
 (re-frame/reg-event-fx
  ::authenticate authenticate)
 
+(defn verify-auth-response
+  "Since we don't get real status codes, we have to look into the server's
+  response and see whether we actually sent the correct credentials"
+  [fx [_ user pass response]]
+  {:dispatch (if (api/is-error? response)
+               [:notification/show :error (api/error-msg (api/->exception response))]
+               [::credentials-verified user pass])})
+
+(re-frame/reg-event-fx
+ ::verify-auth-response
+ verify-auth-response)
+
 (defn try-remember-user
   "Enables skipping the auth request when credentials are saved in the
   local storage; otherwise has no effect"
@@ -51,7 +69,7 @@
 (defn credentials-verified
   "Gets called after the server indicates that the credentials entered by a user
   are correct (see `authenticate`)"
-  [{:keys [db store]} [_event user pass _response]]
+  [{:keys [db store]} [_ user pass]]
   (let [auth {:u user :p pass}
         credentials (merge (:credentials db) auth)]
     {:routes/set-credentials auth
@@ -87,28 +105,32 @@
   (let [creds (:credentials db)]
     (api/url (:server creds) endpoint (merge params (select-keys creds [:u :p])))))
 
-(defn api-request [{:keys [db]} [_ endpoint k params]]
+(defn api-request [{:keys [db]} [_ endpoint params]]
   {:http-xhrio {:method :get
                 :uri (api-url db endpoint params)
                 :response-format (ajax/json-response-format {:keywords? true})
-                :on-success [::api-success k]
-                :on-failure [::api-failure]}})
+                :on-success [:api/good-response]
+                :on-failure [:api/bad-response]}})
 
 (re-frame/reg-event-fx
- :api-request
+ :api/request
  api-request)
 
-(re-frame/reg-event-db
- ::api-success
- (fn [db [_ k response]]
-   ; we "unwrap" the responses
-   (assoc db :response (-> response :subsonic-response k))))
+(defn good-api-response [fx [_ response]]
+  (try
+    (assoc-in fx [:db :response] (api/unwrap-response response))
+    (catch ExceptionInfo e
+      {:dispatch [:notification/show :error (api/error-msg e)]})))
 
-(re-frame/reg-event-db
- ::api-failure
+(re-frame/reg-event-fx
+ :api/good-response
+ good-api-response)
+
+(re-frame/reg-event-fx
+ :api/bad-response
  (fn [db event]
-   (println "api call gone bad; CORS headers missing? check for :status 0" event)
-   db))
+   {:log ["API call gone bad; are CORS headers missing? check for :status 0" event]
+    :dispatch [:notification/show :error "Communication with server failed. Check browser logs for details."]}))
 
 ;; musique
 

src/cljs/airsonic_ui/routes.cljs

@@ -28,16 +28,16 @@
 
 (defmethod route-data ::main
   [route-id params query]
-  [:api-request "getAlbumList2" :albumList2 {:type "recent"
-                                             :size 18}])
+  [:api/request "getAlbumList2" {:type "recent"
+                                 :size 18}])
 
 (defmethod route-data ::artist-view
   [route-id params query]
-  [:api-request "getArtist" :artist (select-keys params [:id])])
+  [:api/request "getArtist" (select-keys params [:id])])
 
 (defmethod route-data ::album-view
   [route-id params query]
-  [:api-request "getAlbum" :album (select-keys params [:id])])
+  [:api/request "getAlbum" (select-keys params [:id])])
 
 ;; shouldn't need to change anything below
 

src/cljs/airsonic_ui/utils/api.cljs

@@ -26,14 +26,27 @@
 (defn is-error? [response]
   (= "failed" (get-in response [:subsonic-response :status])))
 
+(defn- unwrap-response* [response]
+  (-> (:subsonic-response response)
+      (dissoc :status :version)
+      vals
+      first))
+
+(defn ->exception
+  "Takes an erroneous response and makes it a real exception"
+  [response]
+  (let [error (unwrap-response* response)]
+    (ex-info (:message response) error)))
+
 (defn unwrap-response
   "Retrieves the actual response body"
   [response]
   (if (is-error? response)
     (let [error (:error response)]
-      (throw (ex-info (:message response) error)))
-    (-> (get-in response [:subsonic-response])
-        (dissoc :status :version)
-        vals
-        first)))
+      (throw (->exception response)))
+    (unwrap-response* response)))
 
+(defn error-msg
+  [exception-info]
+  (let [{:keys [code message]} (ex-data exception-info)]
+    (str "Error " code ": " message)))

src/cljs/airsonic_ui/views.cljs

@@ -5,6 +5,7 @@
             [airsonic-ui.events :as events]
             [airsonic-ui.subs :as subs]
 
+            [airsonic-ui.views.notifications :refer [notification-list]]
             [airsonic-ui.views.breadcrumbs :refer [breadcrumbs]]
             [airsonic-ui.views.bottom-bar :refer [bottom-bar]]
             [airsonic-ui.views.login :refer [login-form]]
@@ -46,26 +47,12 @@
           {:on-click #(dispatch [::events/initialize-db]) :href "#"}
           (str "Logout (" (:name user) ")")]]]])
 
-;; user notifications
-
-(defn notification-list [notifications]
-  [:div.notifications
-   (for [[id notification] notifications]
-     (let [class (case (:level notification)
-                   :error "danger"
-                   "info")]
-       ^{:key id} [:div {:class-name (str "notification is-small is-" class)}
-                   [:button.delete {:on-click #(dispatch [:notification/hide id])}]
-                   (:message notification)]))])
-
 ;; putting everything together
 
 (defn app [route params query]
   (let [user @(subscribe [::subs/user])
-        notifications @(subscribe [::subs/notifications])
         content @(subscribe [::subs/current-content])]
     [:div
-     [notification-list notifications]
      [:main.columns
       [:div.column.is-2.sidebar
        [sidebar user]]
@@ -79,7 +66,10 @@
      [bottom-bar]]))
 
 (defn main-panel []
-  (let [[route params query] @(subscribe [::subs/current-route])]
-    (case route
-      ::routes/login [login-form]
-      [app route params query])))
+  (let [[route params query] @(subscribe [::subs/current-route])
+        notifications @(subscribe [::subs/notifications])]
+    [:div
+     [notification-list notifications]
+     (case route
+       ::routes/login [login-form]
+       [app route params query])]))

src/cljs/airsonic_ui/views/notifications.cljs

@@ -0,0 +1,14 @@
+(ns airsonic-ui.views.notifications
+  (:require [re-frame.core :refer [dispatch]]))
+
+;; user notifications
+
+(defn notification-list [notifications]
+  [:div.notifications
+   (for [[id notification] notifications]
+     (let [class (case (:level notification)
+                   :error "danger"
+                   "info")]
+       ^{:key id} [:div {:class-name (str "notification is-small is-" class)}
+                   [:button.delete {:on-click #(dispatch [:notification/hide id])}]
+                   (:message notification)]))])

src/sass/app.sass

@@ -77,7 +77,8 @@
 // floating notifications
 .notifications
   @extend .container
-  padding-top: 3.2rem
+  z-index: 100
   position: fixed
   left: 0
   right: 0
+  padding-top: 3.2rem

test/cljs/airsonic_ui/events_test.cljs

@@ -1,6 +1,7 @@
 (ns airsonic-ui.events-test
   (:require [cljs.test :refer [deftest testing is]]
             [clojure.string :as str]
+            [airsonic-ui.fixtures :refer [responses]]
             [airsonic-ui.events :as events]))
 
 (enable-console-print!)
@@ -16,8 +17,14 @@
       (testing "saves the given server location"
         (is (= server (get-in fx [:db :credentials :server]))))
       (testing "invokes correct success callback"
-        (is (= ::events/credentials-verified (first (:on-success request)))))))
-  (testing "On succesfull response"
+        (is (= ::events/verify-auth-response (first (:on-success request)))))))
+  (testing "Auth response verification"
+    (is (= :notification/show
+           (first (:dispatch (events/verify-auth-response {} [:_ "user" "pass" (:error responses)]))))
+        "shows an error when we have an error response")
+    (let [event (:dispatch (events/verify-auth-response {} [:_ "username" "password" (:auth-success responses)]))]
+      (is (= [::events/credentials-verified "username" "password"] event))))
+  (testing "On succesful response"
     (let [creds-before {:server "https://localhost"}
           fx (events/credentials-verified {:db {:credentials creds-before}}
                                           [:_ "user" "pass"])
@@ -47,6 +54,11 @@
 (defn- first-notification [db]
   (-> (:notifications db) vals first))
 
+(deftest api-interaction
+  (testing "Should show an error notification when airsonic responds with an error"
+    (let [fx (events/good-api-response {} [:_ (:error responses)])]
+      (is (= :error (-> fx :dispatch second))))))
+
 (deftest user-notifications
   (testing "Should be able to display a message with an assigned level"
     (is (= :error (:level (first-notification (events/show-notification {} [:_ :error "foo"])))))

test/cljs/airsonic_ui/fixtures.cljs

@@ -0,0 +1,14 @@
+(ns airsonic-ui.fixtures)
+
+(def responses {:error {:subsonic-response
+                        {:error {:code 40
+                                 :message "Wrong username or password"}
+                         :status "failed"
+                         :version "1.15.0"}}
+                :ok {:subsonic-response
+                     {:scanStatus {:count 10326
+                                   :scanning false}
+                      :status "ok"
+                      :version "1.15.0"}}
+                :auth-success {:subsonic-response {:status "ok"
+                                                   :version "1.15.0"}}})

test/cljs/airsonic_ui/utils/api_test.cljs

@@ -1,6 +1,7 @@
 (ns airsonic-ui.utils.api-test
   (:require [cljs.test :refer [deftest testing is]]
             [clojure.string :as str]
+            [airsonic-ui.fixtures :refer [responses]]
             [airsonic-ui.utils.api :as api]))
 
 (defn- url
@@ -9,17 +10,7 @@
   (api/url server endpoint {}))
 
 (def fixtures
-  {:default-url (url "http://localhost:8080" "ping")
-   :responses {:error {:subsonic-response
-                       {:error {:code 40
-                                :message "Wrong username or password"}
-                        :status "failed"
-                        :version "1.15.0"}}}
-               :ok {:subsonic-response
-                    {:scanStatus {:count 10326
-                                  :scanning false}
-                     :status "ok"
-                     :version "1.15.0"}}})
+  {:default-url (url "http://localhost:8080" "ping")})
 
 (deftest general-url-construction
   (testing "Handles missing slashes"
@@ -43,14 +34,14 @@
 
 (deftest response-handling
   (testing "Should unwrap responses"
-    (let [response (get-in fixtures [:responses :ok])]
+    (let [response (:ok responses)]
       (is (= (get-in response [:subsonic-response :scanStatus])
              (api/unwrap-response response)))))
   (testing "Should detect errors"
-    (is (true? (api/is-error? (get-in fixtures [:responses :error]))))
-    (is (false? (api/is-error? (get-in fixtures [:responses :ok])))))
+    (is (true? (api/is-error? (:error responses))))
+    (is (false? (api/is-error? (:ok responses)))))
   (testing "Should throw an informative error when trying to unwrap an erroneous response"
-    (let [error-response (get-in fixtures [:responses :error])]
+    (let [error-response (:error responses)]
       (is (thrown? ExceptionInfo (api/unwrap-response error-response)))
       (try
         (api/unwrap-response error-response)