Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cancellation during client connection establishment #721

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
Fix test-in-flight-request-cancellation test
See code comment for background
DerGuteMoritz committed Apr 19, 2024
commit d2233c649e1e204977da133ba1c4acfd3c91a2b0
16 changes: 10 additions & 6 deletions test/aleph/http_test.clj
Original file line number Diff line number Diff line change
@@ -1480,17 +1480,21 @@
(is (some-> @connect-future .isCancelled)))))))

(deftest test-in-flight-request-cancellation
(let [conn-established (promise)
conn-closed (promise)]
(let [conn-established (atom nil)
conn-closed (atom nil)]
(with-raw-handler (fn [req]
(deliver conn-established true)
(deliver @conn-established true)
(s/on-closed (:body req)
(fn []
(deliver conn-closed true))))
(deliver @conn-closed true))))
;; NOTE: The atom indirection here is needed because `with-raw-handler` will run the body
;; twice (for HTTP1 and HTTP2), so we need a new promise for each run.
(reset! conn-established (promise))
(reset! conn-closed (promise))
(let [rsp (http-get "/")]
(is (= true (deref conn-established 1000 :timeout)))
(is (= true (deref @conn-established 1000 :timeout)))
(http/cancel-request! rsp)
(is (= true (deref conn-closed 1000 :timeout)))
(is (= true (deref @conn-closed 1000 :timeout)))
Comment on lines -1463 to +1497
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(deref @ ... looks weird. I suggest switching to unwrap/unwrap'.

(is (thrown? RequestCancellationException (deref rsp 1000 :timeout)))))))

(deftest ^:leak test-leak-in-raw-stream-handler