master 7de2c20571c 6/8: Handle Lisp errors in Eshell workers

branch: master
commit 7de2c20571c9c0d5c4b5ff1caa0fcf51a1140935
Author: Jim Porter <[email protected]>
Commit: Jim Porter <[email protected]>

    Handle Lisp errors in Eshell workers
    
    * lisp/eshell/esh-worker.el (eshell-worker-name): New generic function.
    (eshell-worker--error): New defsubst.
    (eshell-output-object-to-target, eshell-close-target): Wrap
    'cl-call-next-method' in error handlers.
    
    * test/lisp/eshell/esh-worker-tests.el
    (esh-worker-test/pipe/error-handling)
    (esh-worker-test/map-lines/error-handling)
    (esh-worker-test/apply-lines/error-handling): New tests.
---
 lisp/eshell/esh-worker.el            | 41 ++++++++++++++++++++++++++++++------
 test/lisp/eshell/esh-worker-tests.el | 25 ++++++++++++++++++++++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/lisp/eshell/esh-worker.el b/lisp/eshell/esh-worker.el
index 69319b3f7e7..9c8f613ef82 100644
--- a/lisp/eshell/esh-worker.el
+++ b/lisp/eshell/esh-worker.el
@@ -57,6 +57,8 @@
 
 (require 'esh-io)
 
+(declare-function eshell-set-exit-info "esh-cmd" (status &optional result))
+
 (defgroup eshell-worker nil
   "Eshell workers provide a way to construct process-like objects in Emacs
 Lisp that can serve as pipe targets, allowing you to manipulate another
@@ -106,6 +108,14 @@ task in Eshell."
 This just returns RAW-TARGET."
   raw-target)
 
+(cl-defgeneric eshell-worker-name (_worker)
+  "Return the program name for WORKER."
+  nil)
+
+(defsubst eshell-worker--error (worker error)
+  (eshell-errorn (format "%s: %s" (eshell-worker-name worker)
+                         (error-message-string error))))
+
 (cl-defmethod eshell-output-object-to-target :around
   (_object (target eshell-worker))
   "Output OBJECT to the Eshell worker TARGET.
@@ -116,7 +126,11 @@ for let-binding the proper value for `eshell-current-handles'."
   (let ((eshell-current-handles (eshell-worker-output-handles target))
         (eshell-ensure-newline-p (eshell-worker-ensure-newline-p target)))
     (with-current-buffer (eshell-worker-eshell-buffer target)
-      (cl-call-next-method))
+      (eshell-condition-case err
+          (cl-call-next-method)
+        (error
+         (eshell-worker--error target err)
+         (eshell-set-exit-info 1))))
     (setf (eshell-worker-ensure-newline-p target) eshell-ensure-newline-p)))
 
 (cl-defmethod eshell-close-target :around ((target eshell-worker) _status)
@@ -129,11 +143,17 @@ closing the handles when done, and calling
     (let ((eshell-current-handles (eshell-worker-output-handles target))
           (eshell-ensure-newline-p (eshell-worker-ensure-newline-p target)))
       (with-current-buffer (eshell-worker-eshell-buffer target)
-        (cl-call-next-method)
-        (setf (eshell-worker-status target) 'exit)
-        (eshell-close-handles)
-        (declare-function eshell-kill-process-function "esh-proc" (proc status))
-        (eshell-kill-process-function target "finished\n")))))
+        (unwind-protect
+            (eshell-condition-case err
+                (cl-call-next-method)
+              (error
+               (eshell-worker--error target err)
+               (eshell-set-exit-info 1)))
+          (setf (eshell-worker-status target) 'exit)
+          (eshell-close-handles)
+          (declare-function eshell-kill-process-function "esh-proc"
+                            (proc status))
+          (eshell-kill-process-function target "finished\n"))))))
 
 (defun eshell--apply-print (function args)
   "Call FUNCTION with Eshell-converted ARGS and print the result."
@@ -162,6 +182,9 @@ pass the value unaltered to FUNCTION."
   (setf (eshell-accumulate-worker-buffer-or-value worker)
         (generate-new-buffer " *eshell-worker*" t)))
 
+(cl-defmethod eshell-worker-name ((worker eshell-accumulate-worker))
+  (symbol-name (eshell-accumulate-worker-function worker)))
+
 (cl-defmethod eshell-output-object-to-target
   (object (target eshell-accumulate-worker))
   "Send OBJECT to the accumulate-worker TARGET.
@@ -226,6 +249,9 @@ specified value."
   (let ((function (eshell-map-lines-worker-function target)))
     (eshell--apply-print function (list line))))
 
+(cl-defmethod eshell-worker-name ((worker eshell-map-lines-worker))
+  (format "map-lines %s" (eshell-map-lines-worker-function worker)))
+
 (cl-defmethod eshell-output-object-to-target
   (object (target eshell-map-lines-worker))
   "Send OBJECT to the map-lines worker TARGET.
@@ -277,6 +303,9 @@ other data types to this worker (e.g. lists), each object is passed as a
 single argument to FUNCTION."
   args)                              ; Arguments stored in reverse order
 
+(cl-defmethod eshell-worker-name ((worker eshell-apply-lines-worker))
+  (format "apply-lines %s" (eshell-map-lines-worker-function worker)))
+
 (cl-defmethod eshell-map-lines-worker--apply
     (line (target eshell-apply-lines-worker))
   (push line (eshell-apply-lines-worker-args target)))
diff --git a/test/lisp/eshell/esh-worker-tests.el b/test/lisp/eshell/esh-worker-tests.el
index 7dfca9859cd..b9b1ea3c256 100644
--- a/test/lisp/eshell/esh-worker-tests.el
+++ b/test/lisp/eshell/esh-worker-tests.el
@@ -90,6 +90,14 @@
      "echo hi | #'upcase | (lambda (i) (concat \"> \" i))"
      "\\`> HI\n\\'")))
 
+(ert-deftest esh-worker-test/pipe/error-handling ()
+  "Test that Eshell workers catch errors."
+  (with-temp-eshell
+    (eshell-match-command-output
+     "echo hi | #'1+"
+     "\\`1\\+: Wrong type argument: number-or-marker-p, \"hi\"\n\\'")
+    (should (= eshell-last-command-status 1))))
+
 
 ;; `map-lines' pipelines
 
@@ -151,6 +159,14 @@ It should call the mapped function once per line."
      "{echo '10\n1'; echo '5\n20'} | map-lines #'1+"
      "\\`11\n16\n21\n\\'")))
 
+(ert-deftest esh-worker-test/map-lines/error-handling ()
+  "Test that `map-lines' catches errors."
+  (with-temp-eshell
+    (eshell-match-command-output
+     "echo hi | map-lines #'1+"
+     "\\`map-lines 1\\+: Wrong type argument: number-or-marker-p, \"hi\"\n\\'")
+    (should (= eshell-last-command-status 1))))
+
 
 ;; `apply-lines' pipelines
 
@@ -196,4 +212,13 @@ It should pass each line as an argument to the applied function."
      "{echo '10\n1'; echo '5\n20'} | apply-lines #'+"
      "\\`45\n\\'")))
 
+(ert-deftest esh-worker-test/apply-lines/error-handling ()
+  "Test that `apply-lines' catches errors."
+  (with-temp-eshell
+    (eshell-match-command-output
+     "echo hi | apply-lines #'1+"
+     (concat "\\`apply-lines 1\\+: Wrong type argument: number-or-marker-p, "
+             "\"hi\"\n\\'"))
+    (should (= eshell-last-command-status 1))))
+
 ;;; esh-io-tests.el ends here