emacs-promise
emacs-promise copied to clipboard
chuntaro
fix promise-finally
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")
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)
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.
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
OOPS, promise-error is wrong. It's meant to say promise-reject, sorry.
The diff looks good to me.
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.
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
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
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 https://github.com/chuntaro/emacs-promise/issues/7.