Suggested improvements of promises

[ivan4th]

I had some experience with JavaScript promises in the past and would like to share some possible ways to improve blackbird. After all, async is an important part of in-browser programming and they worked out some rather convenient ways to deal with it.

Some pointers:

• Promises/A+ spec https://promisesaplus.com/
• kriskowal's Q library http://documentup.com/kriskowal/q
• MDN docs on promises https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise

If you agree with some or all of the points below, I'll be glad to contribute some code to actually implement them. Some of the features may affect performance and thus I think should be optional.

1. Clean separation of promise producers and consumers

A possible way to separate such code:

(let ((promise
        (make-promise
         #'(lambda (resolve reject)
             (with-delay (0.3)
               (if ok
                   (funcall resolve 42)
                   (funcall reject 'my-error :format-control "...")))))))
  ...)

Or, with convenience macro:

(let ((promise
        (with-promise (resolve reject)
          (as:with-delay (...)
            (if ok
                (resolve 42)
                (reject 'my-error :format-control "..."))))))
  ...)

Additional advantage, besides improved code structure: when code is organized this way, it's harder to accidentally 'fulfill' a promise multiple times.

finish and signal-error may be kept for a while for compatibility reasons.

2. .then()-like semantics

Both attach and attach-errback should always return a new promise that is fulfilled when the callback finishes. In both cases callbacks should be able to return a new promise, in which case the promise returned from attach or attach-errback is only finished when the promise returned from the callback finishes. (As far as I understand, currently all of this only works for callbacks). When an error (or just a serious-condition) is signalled inside a promise callback or errback, it should be caught causing the promise returned from attach / attach-errback to be rejected (as with signal-error). All this may sound as if it may make debugging harder, but see below.

Promise should not 'accumulate' events, it should be possible to resolve/reject the only once.

Also, maybe attach-errback should be renamed to make it look better. Naming it after Q's .fail() may not be a good idea, need to invent something more appropriate.

3. Debugging aid

Determining what actually happened when looking at an error in the middle of a long promise chain is PITA. Luckily this can be fixed to some extent.

First of all, it's possible to add an option to capture the current stack trace in attach / attach-errback and make it possible to access it from the callback context (joining it with any stack traces captured by attach / attach-errback higher up the chain). With some amount of hacking I think it should be even possible to make SLIME actually display this stack trace tail in sldb buffers, but even just printing it to REPL buffer may help.

Second, there's such thing as .done() in Q. Although it's described as a 'stopgap' in the docs, it may help us, too. See: http://documentup.com/kriskowal/q/#tutorial/the-end The idea is that every callback chain should be topped with .done() (or wrapped in something like (bb:end ...) in our case). This will catch any unhandled conditions that propagated to the end of promise chain and signal them so they can be handled by debugger or some app-level global uncaught error handler.

Another measure is an option to use trivial-garbage to attach finalizer to promise objects so that 'orphaned' promises with unhandled errors are detected and their stack traces are printed during GC. It will require a bit of trickery because promise's finalizer cannot access the promise itself, but nevertheless it's still not that hard to implement. There's also possibility to use repl eval hooks to catch orphaned promises that are created during evaluation of a form entered in REPL, such as (run-tests).

It's possible to also add 'strict' option which will require all promise chains to be explicitly topped with (bb:end ...) and will produce warnings with stack traces if any 'orphaned' promises are detected during GC, with pending unhandled conditions or without.

4. Forced asynchronousness

It should be possible to make all promise-related operations forcibly asynchronous. Although such an option may affect performance, it may help fight unexpected control flow aberrations resulting from some code returning promises under some circumstances and direct values under some other circumstances. Such aberrations may be hard to detect during testing.

This mode of operation should be used when *idle-call-function* is non-null (a possible function to be placed in this variable can be implemented using recently added 'idlers' from cl-async). In this mode, callbacks and errbacks should only be invoked when the control reaches application event loop (not necessarily cl-async event loop, similar things can be done for iolib, CommonQt event loop and so on).

Also, there should be an easy way to convert a plain value to a promise like Q(value) does it, e.g. a macro called (promisify ...) which is an easier way to write

(attach
 (with-promise (accept) (accept t))
 #'(lambda (v)
     (declare (ignore v))
     ...))

which provides delayed (on-idle) execution and promise-based error handling.

5. Error handling

I think promise-handler-case semantics should be cleaned up a bit. Namely, I think that

(promise-handler-case (something)
  (some-error (e)
    (format t "some error: ~a~%" e))
  (another-error (e)
    (format t "another error: ~a~%" e)))

should behave like the following code which assumes the new attach-errback behavior described below:

(attach-errback
 (bb:promisify (something))
 #'(lambda (e)
     (typecase e
       (some-error
        (format t "some error: ~a~%" e))
       (another-error
        (format t "another error: ~a~%" e))
       (t
        (error e)))))

I understand that this substantially differs from what promise-handler-case is currently doing but as of now IMO it provides rather easy way to shoot oneself in the foot. Maybe the new version of error handling form should have some different name. Lexically scoped form of error handling may be useful from performance POV but frankly it's not overly intuitive.

Another useful construct would be something like promise-protect which works like an asynchronous version of unwind-protect or like Q.finally().

6. Misc

Some useful funcs can be borrowed from Q such as Q.resolve(), Q.reject(), Q.all().

[orthecreedence]

This is all great feedback, and is actually one of the main reasons why I forked blackbird from cl-async-future...I've been using Bluebird (similar to Q) in Node.js for my job recently and have been thinking about all the ways that the library can be improved based on my experience. We are thinking very similar things.

This includes convenience functions like all, map, etc as well as separating promise creation (and finishing) from binding (via your with-promise macro). Now that I think about it, attach-errback and promise-handler-case are very similar. If attach-errback (or something new like catcher, name suggestions welcome...fail seems a bit confusing) returned a new promise, it would be possible to feed errors from sub-forms up to the toplevel without wrapping all the attach calls like promise-handler-case currently does (it does a lot of strange macro rewriting which I agree is not that nice AND only propagates lexically). The more correct solution would be to forward errors to the next promise in the chain (and support this in alet or any other form that creates intermediary/hidden futures).

Promise should not 'accumulate' events, it should be possible to resolve/reject the only once.

Agreed! Multi-fire promises are fairly counter intuitive (just use a callback...).

As far as forced asynchronousness, this is something I've read about and I'm in favor of, however the reason it works so great in javascript is because an event loop is always available, no matter where you are. So whatever solution we come up with would need to be very cl-async/event-loop/etc independent, very much like how you suggested with the idler callback, or you could even specify a function that takes a curried callback and finishes the promise when the function is called:

;; default
(setf *promise-async-finisher* (lambda (finish-fn) (funcall finish-fn)))
;; run on next loop
(setf *promise-async-finisher*
  (lambda (finish-fn)
    (as:with-delay () (funcall finish-fn))))

As far as debugging goes, this is something I'd really like help on. I have no knowledge of grabbing stack traces or how to format them, and it's also something that the library is currently horrible at. There is no visibility into failures whatsoever, and this hurts users of the library pretty badly. I'm open to any ideas you have here.

For now, I'm going to start working on with-promise (only allows finishing/rejecting the promise within a lexical form) and also creating a new catch function which returns a new promise as well as getting error chaining more worked out. I may split up what's in here into multiple issues so they're easier to track/discuss as things move along. Probably won't be able to get to this all tonight, but I'll get a start.

Thanks for your feedback, as well as the push I needed to actually get this stuff working.

[ivan4th]

Concerning stack traces: @Shinmera suggested his 'dissect' library when I asked about backtraces on #lisp: https://github.com/Shinmera/dissect (docs at http://shinmera.github.io/dissect/ ), it's also present in quicklisp. Although it only supports CCL and SBCL at the moment, I think it's better than nothing as backtraces aren't critical for program execution and are only needed for debugging / error reporting.

[Shinmera]

As I mentioned in the chat, if there's any implementation that you specifically need support for I can put that onto priority and get to writing the specific parts to dissect sooner.

[orthecreedence]

Dissect looks really promising. BTW, just made a big commit (4347ed68) which addresses a lot of the items above. I need to document everything, but for now here's a quick rundown (there's a lot more detail in the commit log):

• error handling is improved a lot, the concept of chaining errors is now as important as chaining values in promises. a top-level error handler (now called catcher) has access to errors deep down in the promise chain.
• added finally, whos form is called whether the given promise has an error or a value.
• promise creation is now done with via with-promise, which provides resolve/reject functions and catches errors in the given body form.
• adding some helpers: amap, areduce, all
• adding chaining macro to make simple operations a bit easier:

(chain 4
  (:then (x) (+ 5 x))
  (:then (x) (list x (+ x 3) (* x 2)))
  (:reduce (acc x 0) (+ acc x))
  (:then (y) (format t "final val ~a~%" y)))
;; prints "final val 39"

[orthecreedence]

Added *promise-finish-hook* in 605414f5b0e44fc96dcfe67a3932c1b1c6c63b4d for forcing async finishing:

(setf *promise-finish-hook* (lambda (finish-fn) (as:delay finish-fn)))

I think at this point, the only thing left to do is improve debugging (unless I missed something in the last few commits). I've added a new issue to track this, as well as the possibility of detecting unfinished/uncaught/unchained promises.

For now I'm closing this issue. However, we can still use it as a place to discuss improvements/changes.

[ivan4th]

That's just great! Thank you for your work!

[orthecreedence]

Hey, a quick update. Just changed the way resolve works in with-promise:

If multiple values are given, the promise is finished with multiple values: (resolve 1 2 3). If a single value is given, the promise is finished with the value(s) of that form: (resolve (values 1 2 3))

So this means that:

(with-promise (resolve reject) (resolve 1 2 3))
(with-promise (resolve reject) (resolve (values 1 2 3))
(with-promise (resolve reject) (funcall (lambda () (values 1 2 3))))

will all result in the same promise (a promise of three values: 1, 2, 3).

I think this makes things a bit easier, because you don't have to manually capture multiple values and apply them to the :resolve-fn.