From ed6636b8e2dd930bcd2a6b92cfad5902447b62d1 Mon Sep 17 00:00:00 2001 From: Moritz Heidkamp Date: Wed, 17 Sep 2025 22:12:41 +0200 Subject: [PATCH] Fix dropped errors in `manifold.deferred/timeout!` When putting the result deferred `d` in an error state, deferred returned by `chain` would be detected as a dropped error because it's only used to attach a side-effecting callback for cancelling the timeout. The fix then is to use `on-realized` instead to attach a handler for both cases. As a consequence, timeouts are now also properly cancelled when the result deferred is put in an error state, freeing up resources in a more timely fashion. This fixes one of the causes of https://github.com/clj-commons/aleph/issues/766, as well as 2 of the 3 dropped errors in `manifold.go-off-test`. Also, add dedicated tests for `d/timeout!`. --- src/manifold/deferred.clj | 16 +++++++++++----- test/manifold/deferred_test.clj | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/manifold/deferred.clj b/src/manifold/deferred.clj index 4685469..8b7e84d 100644 --- a/src/manifold/deferred.clj +++ b/src/manifold/deferred.clj @@ -1293,9 +1293,12 @@ (let [timeout-d (time/in interval #(error! d (TimeoutException. - (str "timed out after " interval " milliseconds"))))] - (chain d (fn [_] - (success! timeout-d true))))) + (str "timed out after " interval " milliseconds"))))] + (on-realized d + (fn [_] + (success! timeout-d true)) + (fn [_] + (success! timeout-d false))))) d) ([d interval timeout-value] (cond @@ -1308,8 +1311,11 @@ :else (let [timeout-d (time/in interval #(success! d timeout-value))] - (chain d (fn [_] - (success! timeout-d true))))) + (on-realized d + (fn [_] + (success! timeout-d true)) + (fn [_] + (success! timeout-d false))))) d)) (deftype+ Recur [s] diff --git a/test/manifold/deferred_test.clj b/test/manifold/deferred_test.clj index 9475afa..91fd614 100644 --- a/test/manifold/deferred_test.clj +++ b/test/manifold/deferred_test.clj @@ -239,6 +239,28 @@ (is (= 1 @d)) (is (= 1 (deref d 10 :foo))))) +(deftest test-timeout + (testing "exception by default" + (let [d (d/deferred) + t (d/timeout! d 1)] + (is (identical? d t)) + (is (thrown-with-msg? TimeoutException + #"^timed out after 1 milliseconds$" + @d)))) + + (testing "custom default value" + (let [d (d/deferred) + t (d/timeout! d 1 ::timeout)] + (is (identical? d t)) + (is @(capture-success d ::timeout)))) + + (testing "error before timeout" + (let [ex (Exception.) + d (d/deferred) + t (d/timeout! d 1000)] + (d/error! d ex) + (is (= ex (deref (capture-error t) 100 ::error)))))) + (deftest test-loop ;; body produces a non-deferred value (is @(capture-success