refactoring, error handling

Eask

@@ -21,4 +21,4 @@
 
 (source "melpa")
 
-(depends-on "emacs" "26.1")
+(depends-on "emacs" "27.1")

reel.el

@@ -9,7 +9,7 @@
 ;; Version: 0.1.0
 ;; Keywords: lisp
 ;; Homepage: https://github.com/rfaulhaber/reel
-;; Package-Requires: ((emacs "25.2"))
+;; Package-Requires: ((emacs "27.1"))
 ;;
 ;; This file is not part of GNU Emacs.
 ;;
@@ -72,6 +72,9 @@
   "A reusable reel client."
   (ptr nil :read-only t))
 
+(define-error 'reel-invalid-body-type "the :body keyword of a reel request must be a string or an alist")
+(define-error 'reel-invalid-method "invalid HTTP method passed to :method")
+
 ;;;###autoload
 (cl-defun reel (url-or-request &key method headers body)
   "Make an HTTP request with URL-OR-REQUEST.
@@ -106,15 +109,40 @@ key-value pairs for form submission.
 
 (defun reel-url-search-parameters (parameters)
   "Given an alist PARAMETERS, converts the alist to a search query string."
-  (concat "?"
-          (string-join
-           (seq-map (lambda (param)
-                      (concat (car param) "=" (cdr param)))
-                    parameters)
-           "&")))
+  (when (not (proper-list-p parameters))
+    (user-error "PARAMETERS must be a proper alist"))
+
+  (if (null parameters)
+      ""
+    (concat "?"
+            (string-join
+             (seq-map (lambda (param)
+                        (concat (car param) "=" (cdr param)))
+                      parameters)
+             "&"))))
+
+(defun reel-valid-body-p (body)
+  "Returns non-nil if BODY is valid for the `reel' function :body keyword."
+  (or (stringp body)
+      (proper-list-p body)))
+
+(defun reel-valid-method-p (method)
+  "Returns non-nil if METHOD is a valid HTTP method."
+  (let ((canonical-method (if (stringp method)
+                              (intern (upcase method))
+                            method)))
+    (member canonical-method reel-http-methods)))
 
 (defun reel--make-request (client url method headers body)
   "Makes a request via reel-dyn using defaults."
+  (when (and (not (null body))
+             (not (reel-valid-body-p body)))
+    (signal 'reel-invalid-body-type `(reel-valid-body-p ,(type-of body))))
+
+  (when (and (not (null method))
+             (not (reel-valid-method-p method)))
+    (signal 'reel-invalid-method `(reel-valid-method-p ,method)))
+
   (let* ((client (if (reel-client-p client) (reel-client-ptr client) client))
          (method (or method "GET"))
          (default-headers (if (assoc "user-agent" headers) headers (push `("user-agent" . ,reel-default-user-agent) headers)))

src/lib.rs

@@ -5,10 +5,6 @@ use tokio::runtime;
 
 emacs::plugin_is_GPL_compatible!();
 
-emacs::define_errors! {
-    request_body_type_error "Request body should either be a string or a cons cell for form data"
-}
-
 #[derive(Debug)]
 pub struct ConsCell<T, V>
 where
@@ -32,7 +28,11 @@ where
     }
 }
 
-impl<T: for<'t> emacs::FromLisp<'t>, V: for<'v> emacs::FromLisp<'v>> ConsCell<T, V> {
+impl<T, V> ConsCell<T, V>
+where
+    T: for<'t> emacs::FromLisp<'t>,
+    V: for<'v> emacs::FromLisp<'v>,
+{
     pub fn car(&self) -> &T {
         &self.car
     }
@@ -46,6 +46,16 @@ impl<T: for<'t> emacs::FromLisp<'t>, V: for<'v> emacs::FromLisp<'v>> ConsCell<T,
     }
 }
 
+impl<'e, T, V> From<ConsCell<T, V>> for (T, V)
+where
+    T: for<'t> emacs::FromLisp<'t>,
+    V: for<'v> emacs::FromLisp<'v>,
+{
+    fn from(val: ConsCell<T, V>) -> Self {
+        (val.car, val.cdr)
+    }
+}
+
 #[derive(Debug)]
 pub struct ReelResponse {
     pub body: String,
@@ -67,7 +77,7 @@ pub enum Body {
 
 impl<'e> FromLisp<'e> for Body {
     fn from_lisp(value: emacs::Value<'e>) -> EmacsResult<Self> {
-        let list_value = value.env.call("listp", [value])?;
+        let list_value = value.env.call("proper-list-p", [value])?;
 
         if list_value.is_not_nil() {
             let mut vec = Vec::new();
@@ -76,8 +86,7 @@ impl<'e> FromLisp<'e> for Body {
             let len = env.call("length", [value])?.into_rust::<usize>()?;
 
             for i in 0..len {
-                let nth: ConsCell<String, String> =
-                    env.call("nth", (i, value))?.into_rust()?;
+                let nth: ConsCell<String, String> = env.call("nth", (i, value))?.into_rust()?;
                 vec.push(nth)
             }
 
@@ -92,9 +101,11 @@ impl<'e> FromLisp<'e> for Body {
 
         let received_type = value.env.call("type-of", [value])?;
 
+        // this validation is done in lisp and this shouldn't occur
+        // TODO refactor
         return value
             .env
-            .signal(request_body_type_error, ("type-of", received_type));
+            .signal(value.env.intern("reel-invalid-body-type")?, ("reel-valid-body-p", received_type));
     }
 }
 
@@ -195,7 +206,7 @@ fn make_client_request(
             Some(Body::Form(cells)) => {
                 let pairs: Vec<(String, String)> = cells
                     .into_iter()
-                    .map(|cell| (cell.car().clone(), cell.cdr().clone()))
+                    .map(|cell| cell.into())
                     .collect();
 
                 request.form(&pairs).send().await