missionary 2022-01-12 | Slack Archive

mjmeintjes05:01:06

I just noticed that the new cancel exception inherits from Throwable. What's the reasoning for not inheriting from Exception?

leonoel08:01:28

because when you need to recover from a domain exception, it is very unlikely that you want to recover from cancellation too

leonoel08:01:02

this is an empiric claim, feel free to challenge it

mjmeintjes10:01:18

That makes sense. But how do you deal with InterruptedException or other types of cancellations? Currently in my exception handling, I have a

(catch missionary.Cancelled
  DEAL WITH CANCELLATION)
(catch Exception ex
  (when (ex-cancelled? ex)
    DEAL WITH CANCELLATION))

which leads to duplication. I doesn't feel right to catch Throwable, because that catches other more serious exceptions as well. The other option I can think of is to wrap all the via blocks and return a Cancelled exception when the thread is interrupted.

leonoel20:01:51

If an expression can throw many kinds of exceptions on thread interruption, I would say make it consistent ASAP, preferably on a fine-grained basis. > I doesn't feel right to catch Throwable, because that catches other more serious exceptions as well. Catching Exception has the same problem, it includes many serious errors (e.g RuntimeExceptions). I think it's OK to catch with a broad type like Throwable as long as you rethrow what you don't recover :

(catch Throwable ex
  (if (ex-cancelled? ex)
    (DEAL WITH CANCELLATION)
    (throw ex)))

mjmeintjes21:01:57

I think the difference between catching Exception and Throwable is that Throwable includes irrecoverable errors like out of memory/stack overflow. Would it make sense to wrap InterruptedException inside Cancelled within missionary? Currently it feels inconsistent - you can never be sure that missionary.Cancelled is the only cancellation exception you have to catch. For example:

(m/?
   (m/reduce
    conj
    (m/ap
     (let [n (m/?< (m/seed [1 2]))]
       (try
         (m/? (m/sleep 1000))
         n
         (catch missionary.Cancelled _
           (println "cancelled")))))))

works, but

(m/?
   (m/reduce
    conj
    (m/ap
     (let [n (m/?< (m/seed [1 2]))]
       (try
         (m/? (m/via
               m/cpu
               (Thread/sleep 1000)))
         n
         (catch missionary.Cancelled _
           (println "cancelled")))))))

does not work. In effect you always have to wrap m/via in a try/catch and throw missionary.Cancelled, otherwise the code does not work as expected.

mjmeintjes21:01:43

To deal with this issue, I have changed my code to use custom via macros instead of the missionary via macros, something like:

(defmacro via-blk [& body]
  `(ms/via ms/blk
           (try
             ~@body
             (catch Exception ex#
               (if (ex-cancelled? ex#)
                 (throw (missionary.Cancelled. (ex-message ex#)))
                 (throw ex#))))))

Does it make sense to do it this way?

leonoel21:01:26

> Does it make sense to do it this way? I think it does. > Would it make sense to wrap InterruptedException inside Cancelled within missionary? I considered that, via is about thread interop so it could intercept InterruptedException and turn it into missionary.Cancelled. Similarly, ? in thread mode could intercept missionary.Cancelled and turn it into InterruptedException. The problem is, nobody is forced to use InterruptedException, and in practice there's very little consistency in how blocking code reacts to thread interruption (this is the essence of your problem, if I understand correctly). Therefore, I think it's a reasonable choice for missionary not to perform any magic conversion and leave full control to the user. > Currently it feels inconsistent - you can never be sure that missionary.Cancelled is the only cancellation exception you have to catch. In general, you can't be sure indeed. But here the inconsistency comes from the code you need to interop with, and only you can make the right decision about it. On the same topic you can check https://github.com/leonoel/missionary/discussions/57

2022-01-12

Channels