fix promise-finally

[bendlas]

make promise-finally rethrow on error, as per spec

[chuntaro]

Thank you for the report.
Instead of PullReq's method, I have committed a fix, is this not a problem?

"use strice";

function timeoutPromise(time, reason) {
  return new Promise(function(resolve, reject) {
    setTimeout(function() {
      reject(reason);
    }, time * 1000);
  });
}

timeoutPromise(0.2, "too late")
  .then(function(value) {
    console.log("value: %d", value);
  })
  .catch(function(reason) {
    console.log("!!! %o !!!", reason);
  })
  .finally(function() {
    console.log("Done! (1)");
  })
  .catch(function(reason) {
    console.log("!!! %o !!!", reason);
  });

timeoutPromise(0.4, "too late")
  .then(function(value) {
    console.log("value: %d", value);
  })
  // .catch(function(reason) {
  //   console.log("!!! %o !!!", reason);
  // })
  .finally(function() {
    console.log("Done! (2)");
  })
  .catch(function(reason) {
    console.log("!!! %o !!!", reason);
  });

timeoutPromise(0.6, "too late")
  .then(function(value) {
    console.log("value: %d", value);
  })
  // .catch(function(reason) {
  //   console.log("!!! %o !!!", reason);
  // })
  .finally(function() {
    console.log("Done! (3)");
  });

$ node test.js
!!! 'too late' !!!
Done! (1)
Done! (2)
!!! 'too late' !!!
Done! (3)
(node:3792) UnhandledPromiseRejectionWarning: too late
(node:3792) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3)
(node:3792) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

;;; -*- lexical-binding: t; -*-

(require 'promise)
(promise-rejection-tracking-enable '((all-rejections . t)))

(promise-chain (promise:time-out 0.2 "too late")
  (then (lambda (value)
          (message "value: %d" value)))
  (promise-catch (lambda (err)
                   (message "!!! %s !!!" err)))
  (promise-finally (lambda ()
                     (message "Done! (1)")))
  (promise-catch (lambda (err)
                   (message "!!! %s !!!" err))))

(promise-chain (promise:time-out 0.4 "too late")
  (then (lambda (value)
          (message "value: %d" value)))
  ;; (promise-catch (lambda (err)
  ;;                  (message "!!! %s !!!" err)))
  (promise-finally (lambda ()
                     (message "Done! (2)")))
  (promise-catch (lambda (err)
                   (message "!!! %s !!!" err))))

(promise-chain (promise:time-out 0.6 "too late")
  (then (lambda (value)
          (message "value: %d" value)))
  ;; (promise-catch (lambda (err)
  ;;                  (message "!!! %s !!!" err)))
  (promise-finally (lambda ()
                     (message "Done! (3)"))))

(cl-loop repeat 50 do (sit-for 0.1))    ; Waiting for Promise resolved

Before fix

$ emacs --batch --eval "(package-initialize)" -l test.el
!!! too late !!!
Done! (1)
Done! (2)
Done! (3)

After fix

$ emacs --batch --eval "(package-initialize)" -l test.el
!!! too late !!!
Done! (1)
Done! (2)
!!! (error too late) !!!
Done! (3)
Warning (promise): Possible Unhandled Promise Rejection (id:0):
Warning (promise): (error "too late")

[bendlas]

Instead of PullReq's method, I have committed a fix, is this not a problem?

No problem at all, except that your fix doesn't seem to be right when dealing with other data than errors on the rejection path:

ELISP> (promise-then (promise-reject :nope)
                     (lambda (val) (message "VAL: %s" val))
                     (lambda (err) (message "ERR: %s" err)))
ERR: :nope

ELISP> (promise-then (promise-finally (promise-reject :nope)
                                      (lambda () (message "Final Steps")))
                     (lambda (val) (message "VAL: %s" val))
                     (lambda (err) (message "ERR: %s" err)))
Final Steps
ERR: (wrong-type-argument stringp :nope)

[bendlas]

Btw, not tested, but I think promise-done suffers from the same problem.

Remember, that in JS, you can throw and catch any value, where as in elisp, error only takes strings.

[chuntaro]

Fix to promise-reject simply without throwing an error in promise-finally and promise-done.
It's different from JavaScript's way of throwing errors, but the results are better.

What kind of function is PullReq's promise-error?
Is it the same as promise-reject?

diff --git a/promise-done.el b/promise-done.el
index dd7ffab..6968020 100644
--- a/promise-done.el
+++ b/promise-done.el
@@ -57,9 +57,7 @@
     (promise-then self nil (lambda (err)
                              (run-at-time 0 nil
                                           (lambda ()
-                                            (if (listp err)
-                                                (signal (car err) (cdr err))
-                                              (error err)))))))
+                                            (promise-reject err))))))
   nil)
 
 (provide 'promise-done)
diff --git a/promise-finally.el b/promise-finally.el
index e9aecc6..bf8c731 100644
--- a/promise-finally.el
+++ b/promise-finally.el
@@ -58,9 +58,7 @@
                 (lambda (err)
                   (promise-then (promise-resolve (funcall f))
                                 (lambda (_)
-                                  (if (consp err)
-                                      (signal (car err) (cdr err))
-                                    (error err)))))))
+                                  (promise-reject err))))))
 
 (provide 'promise-finally)
 ;;; promise-finally.el ends here

[bendlas]

OOPS, promise-error is wrong. It's meant to say promise-reject, sorry.
The diff looks good to me.

[bendlas]

The diff looks good to me.

Actually, the diff for promise-done doesn't seem right: returning a promise-reject from a timer function won't do anything.

I think, the throw in Promise.done is just supposed to do error reporting. Maybe we need some custom error reporting there ...

promise-finally still looking good. It's the version I'm currently using, actually.

[chuntaro]

Sorry, I misunderstood the behavior of promise-done.
Is it okay to commit the following fixes?

diff --git a/promise-done.el b/promise-done.el
index dd7ffab..d787087 100644
--- a/promise-done.el
+++ b/promise-done.el
@@ -57,9 +57,9 @@
     (promise-then self nil (lambda (err)
                              (run-at-time 0 nil
                                           (lambda ()
-                                            (if (listp err)
+                                            (if (consp err)
                                                 (signal (car err) (cdr err))
-                                              (error err)))))))
+                                              (signal 'error (list err))))))))
   nil)
 
 (provide 'promise-done)
diff --git a/promise-finally.el b/promise-finally.el
index e9aecc6..bf8c731 100644
--- a/promise-finally.el
+++ b/promise-finally.el
@@ -58,9 +58,7 @@
                 (lambda (err)
                   (promise-then (promise-resolve (funcall f))
                                 (lambda (_)
-                                  (if (consp err)
-                                      (signal (car err) (cdr err))
-                                    (error err)))))))
+                                  (promise-reject err))))))
 
 (provide 'promise-finally)
 ;;; promise-finally.el ends here

[bendlas]

for promise done, going off from your version, I think

(if (and (consp err)
         (symbolp (car err)))
    (signal (car err) (cdr err))
  (signal 'error (if (consp err)
                     err
                   (list err))))

would work best ok, but maybe we can have a separate ticket for that ...

EDIT as hinted in the ticket, I think this version of promise-done would do just fine for now:

(cl-defmethod promise-done ((this promise-class) &optional on-fulfilled on-rejected)
  (let ((self (if (or on-fulfilled on-rejected)
                  (promise-then this on-fulfilled on-rejected)
                this)))
    (promise-then self nil (lambda (err)
                             (message "promise-reject: %s" err))))
  nil)

promise-finally LGTM

[chuntaro]

diff --git a/promise-done.el b/promise-done.el
index dd7ffab..8a9a83f 100644
--- a/promise-done.el
+++ b/promise-done.el
@@ -57,9 +57,7 @@
     (promise-then self nil (lambda (err)
                              (run-at-time 0 nil
                                           (lambda ()
-                                            (if (listp err)
-                                                (signal (car err) (cdr err))
-                                              (error err)))))))
+                                            (signal 'error (list err)))))))
   nil)
 
 (provide 'promise-done)
diff --git a/promise-finally.el b/promise-finally.el
index e9aecc6..bf8c731 100644
--- a/promise-finally.el
+++ b/promise-finally.el
@@ -58,9 +58,7 @@
                 (lambda (err)
                   (promise-then (promise-resolve (funcall f))
                                 (lambda (_)
-                                  (if (consp err)
-                                      (signal (car err) (cdr err))
-                                    (error err)))))))
+                                  (promise-reject err))))))
 
 (provide 'promise-finally)
 ;;; promise-finally.el ends here

It seems that there is no need to use message, as this fix results in the following:

(promise-chain (promise-reject '(complex (data)))
  (promise-done (lambda (val)
                  (message "VAL: %s" val))))
=> The following message is displayed in *Messages*.
Error running timer: (error (complex (data)))

The role of promise-done is to drop the error from Promise and turn it into an Emacs error, so this seems good.

This continuation is #7.