LRS-61 Remove Error Interceptor

src/dev/mem_lrs/service.cljc

@@ -2,10 +2,12 @@
   (:require [io.pedestal.http :as http]
             [com.yetanalytics.lrs.impl.memory :as lrs-impl :refer [new-lrs]]
             [com.yetanalytics.lrs.pedestal.routes :refer [build]]
+            [com.yetanalytics.lrs.pedestal.interceptor :as i]
             #?(:cljs [com.yetanalytics.node-chain-provider :as provider])))
 
 (defn new-routes [lrs]
-  (build {:lrs lrs}))
+  (build {:lrs lrs
+          :wrap-interceptors [i/error-interceptor]}))
 
 ;; Tabular routes + default LRS
 (def default-lrs

src/main/com/yetanalytics/lrs.cljc

@@ -1,5 +1,5 @@
 (ns com.yetanalytics.lrs
-  (:require [com.yetanalytics.lrs.protocol :as p]
+  (:require [com.yetanalytics.lrs.protocol :as p :include-macros true]
             [clojure.spec.alpha :as s :include-macros true]
             [xapi-schema.spec :as xs]
             [xapi-schema.spec.resources]
@@ -20,7 +20,8 @@
 (defn get-about
   "Get information about this LRS"
   [lrs auth-identity]
-  (p/-get-about lrs auth-identity))
+  (p/try-sync
+   (p/-get-about lrs auth-identity)))
 
 (s/fdef get-about
   :args (s/cat :lrs           (with-lrs-gen ::p/about-resource-instance)
@@ -43,7 +44,8 @@
 
 (defn set-document
   [lrs auth-identity params document merge?]
-  (p/-set-document lrs auth-identity params document merge?))
+  (p/try-sync
+   (p/-set-document lrs auth-identity params document merge?)))
 
 (s/fdef set-document
   :args (s/cat :lrs           (with-lrs-gen ::p/document-resource-instance)
@@ -68,7 +70,8 @@
 
 (defn get-document
   [lrs auth-identity params]
-  (p/-get-document lrs auth-identity params))
+  (p/try-sync
+   (p/-get-document lrs auth-identity params)))
 
 (s/fdef get-document
   :args (s/cat :lrs           (with-lrs-gen ::p/document-resource-instance)
@@ -89,7 +92,8 @@
 
 (defn get-document-ids
   [lrs auth-identity params]
-  (p/-get-document-ids lrs auth-identity params))
+  (p/try-sync
+   (p/-get-document-ids lrs auth-identity params)))
 
 (s/fdef get-document-ids
   :args (s/cat :lrs           (with-lrs-gen ::p/document-resource-instance)
@@ -110,7 +114,8 @@
 
 (defn delete-document
   [lrs auth-identity params]
-  (p/-delete-document lrs auth-identity params))
+  (p/try-sync
+   (p/-delete-document lrs auth-identity params)))
 
 (s/fdef delete-document
  :args (s/cat :lrs           (with-lrs-gen ::p/document-resource-instance)
@@ -130,7 +135,8 @@
 
 (defn delete-documents
   [lrs auth-identity params]
-  (p/-delete-documents lrs auth-identity params))
+  (p/try-sync
+   (p/-delete-documents lrs auth-identity params)))
 
 (s/fdef delete-documents
  :args (s/cat :lrs           (with-lrs-gen ::p/document-resource-instance)
@@ -156,7 +162,8 @@
 (defn get-activity
   "Get the canonical representation of an activity"
   [lrs auth-identity params]
-  (p/-get-activity lrs auth-identity params))
+  (p/try-sync
+   (p/-get-activity lrs auth-identity params)))
 
 (s/fdef get-activity
   :args (s/cat :lrs           (with-lrs-gen ::p/activity-info-resource-instance)
@@ -184,7 +191,8 @@
 (defn get-person
   "Get an object representing an actor"
   [lrs auth-identity params]
-  (p/-get-person lrs auth-identity params))
+  (p/try-sync
+   (p/-get-person lrs auth-identity params)))
 
 (s/fdef get-person
   :args (s/cat :lrs           (with-lrs-gen ::p/agent-info-resource-instance)
@@ -214,7 +222,8 @@
 (defn store-statements
   "Store statements and attachments in the LRS"
   [lrs auth-identity statements attachments]
-  (p/-store-statements lrs auth-identity statements attachments))
+  (p/try-sync
+   (p/-store-statements lrs auth-identity statements attachments)))
 
 (s/fdef store-statements
   :args (s/cat :lrs           (with-lrs-gen ::p/statements-resource-instance)
@@ -239,7 +248,8 @@
 (defn get-statements
   "Get statements from the LRS"
   [lrs auth-identity params ltags]
-  (p/-get-statements lrs auth-identity params ltags))
+  (p/try-sync
+   (p/-get-statements lrs auth-identity params ltags)))
 
 (s/fdef get-statements
   :args (s/cat :lrs           (with-lrs-gen ::p/statements-resource-instance)
@@ -291,7 +301,8 @@
 (defn authenticate
   "Given the LRS and context, return an identity or nil (401)"
   [lrs ctx]
-  (p/-authenticate lrs ctx))
+  (p/try-sync
+   (p/-authenticate lrs ctx)))
 
 (s/fdef authenticate
   :args (s/cat :lrs (with-lrs-gen ::p/lrs-auth-instance)
@@ -302,7 +313,8 @@
   "Given the LRS and context, return true if the user is allowed to do a given
    thing."
   [lrs ctx auth-identity]
-  (p/-authorize lrs ctx auth-identity))
+  (p/try-sync
+   (p/-authorize lrs ctx auth-identity)))
 
 (s/fdef auth
   :args (s/cat :lrs           (with-lrs-gen ::p/lrs-auth-instance)

src/main/com/yetanalytics/lrs/pedestal/interceptor.cljc

@@ -243,22 +243,24 @@
           ;; Log all unhandled/bubbled errors
           (log/error :msg "Unhandled LRS Error"
                      :exception ex)
-          (let [err-type
-                (cond
-                  (nil? ex)    {:name "unknown"}
-                  (exi? ex)    (let [{:keys [type exception-type]} (ex-data ex)
-                                     type-k (or exception-type
-                                                type
-                                                :unknown/unknown)
-                                     [tns tname] ((juxt namespace name) type-k)]
-                                 (merge (when tns {:ns tns})
-                                        {:name tname}))
-                  (error? ex)  {:name (str (type ex))}
-                  (string? ex) {:name ex}
-                  :else        {:name "unknown"})]
-            (assoc ctx :response {:status 500
-                                  :body   {:error
-                                           {:type err-type}}})))))}))
+          (assoc ctx
+                 :response
+                 {:status 500
+                  :body {:error
+                         (merge
+                          {:message "Unhandled LRS Error"}
+                          (cond
+                            (nil? ex)    {:type "unknown"}
+                            (exi? ex)    (let [{:keys [type exception-type]} (ex-data ex)
+                                               type-k (or exception-type
+                                                          type
+                                                          :unknown/unknown)
+                                               [tns tname] ((juxt namespace name) type-k)]
+                                           (merge (when tns {:ns tns})
+                                                  {:type tname}))
+                            (error? ex)  {:type (str (type ex))}
+                            (string? ex) {:type ex}
+                            :else        {:type "unknown"}))}}))))}))
 
 ;; Time Requests
 
@@ -420,7 +422,6 @@
            :cljs (body-params/body-params))]
     [x-forwarded-for-interceptor
      http/json-body
-     error-interceptor
      body-params
      xapi/alternate-request-syntax-interceptor
      set-xapi-version-interceptor

src/main/com/yetanalytics/lrs/pedestal/interceptor/xapi/statements.cljc

@@ -258,9 +258,13 @@
                    [:response :headers "X-Experience-API-Consistent-Through"]
                    (a/<! (lrs/consistent-through-async lrs ctx auth-identity))))
        (lrsp/statements-resource? lrs)
-       (assoc-in ctx
-                 [:response :headers "X-Experience-API-Consistent-Through"]
-                 (lrs/consistent-through lrs ctx auth-identity))
+       (try (assoc-in ctx
+                      [:response :headers "X-Experience-API-Consistent-Through"]
+                      (lrs/consistent-through lrs ctx auth-identity))
+            (catch #?(:clj Exception :cljs js/Error) ex
+              (assoc ctx
+                     :io.pedestal.interceptor.chain/error
+                     ex)))
        :else
        (assoc-in ctx
                  [:response :headers "X-Experience-API-Consistent-Through"]

src/main/com/yetanalytics/lrs/pedestal/routes.cljc

@@ -85,22 +85,40 @@
     :enter (fn health-fn [ctx]
              (assoc ctx :response {:status 200 :body ""}))}))
 
-(defn build [{:keys [lrs path-prefix]
-                     :or {path-prefix "/xapi"}}]
+(defn build
+  "Given a map with :lrs implementation, builds and returns xAPI routes
+  in pedestal table format.
+
+  Optional keys:
+    :path-prefix - defines the prefix from root for xAPI routes, default /xapi
+    :wrap-interceptors - a vector of interceptors to apply to every route.
+      The default vector includes an error interceptor which should be replaced
+      if this setting is provided."
+  [{:keys [lrs
+           path-prefix
+           wrap-interceptors]
+    :or {path-prefix "/xapi"
+         wrap-interceptors [i/error-interceptor]}}]
   (let [lrs-i                       (i/lrs-interceptor lrs)
-        global-interceptors-no-auth (conj i/common-interceptors
-                                          lrs-i)
-        global-interceptors         (conj i/common-interceptors
-                                          lrs-i
-                                          auth-i/lrs-authenticate
-                                          auth-i/lrs-authorize)
-        protected-interceptors      (into global-interceptors
-                                          i/xapi-protected-interceptors)
-        document-interceptors       (into (conj i/doc-interceptors-base
+        global-interceptors-no-auth (into wrap-interceptors
+                                          (conj i/common-interceptors
+                                                lrs-i))
+        global-interceptors         (into wrap-interceptors
+                                          (conj i/common-interceptors
                                                 lrs-i
                                                 auth-i/lrs-authenticate
-                                                auth-i/lrs-authorize)
-                                          i/xapi-protected-interceptors)]
+                                                auth-i/lrs-authorize))
+        protected-interceptors      (into wrap-interceptors
+                                          (concat
+                                           global-interceptors
+                                           i/xapi-protected-interceptors))
+        document-interceptors       (into wrap-interceptors
+                                          (concat
+                                           (conj i/doc-interceptors-base
+                                                 lrs-i
+                                                 auth-i/lrs-authenticate
+                                                 auth-i/lrs-authorize)
+                                           i/xapi-protected-interceptors))]
     (into #{;; health check
             ["/health"
              :get (conj global-interceptors-no-auth
@@ -116,13 +134,14 @@
 
             ;; xapi statements
             [(format "%s/statements" path-prefix)
-             :get (into [auth-i/www-authenticate]
-                        (concat
-                         protected-interceptors
-                         [statements-i/set-consistent-through
-                          (xapi-i/params-interceptor
-                           :xapi.statements.GET.request/params)
-                          statements/handle-get]))]
+             :get (into
+                   [auth-i/www-authenticate]
+                   (concat
+                    protected-interceptors
+                    [statements-i/set-consistent-through
+                     (xapi-i/params-interceptor
+                      :xapi.statements.GET.request/params)
+                     statements/handle-get]))]
             [(format "%s/statements" path-prefix)
              :head (conj protected-interceptors
                          statements-i/set-consistent-through

src/main/com/yetanalytics/lrs/protocol.cljc

@@ -22,6 +22,13 @@
   `(memoize (fn [x#]
               (satisfies? ~protocol x#))))
 
+(defmacro try-sync
+  "Try body and wrap anything thrown in a map with :error key"
+  [& body]
+  `(try ~@body
+        (catch #?(:clj Exception :cljs js/Error) ex#
+          {:error ex#})))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Error
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;