Skip to content

Commit b04da76

Browse files
committed
Drop wrap-future's "success" treatment of ClosedChannelException
Treating channel closes as successful in the deferred returned by `wrap-future` is a bit of a footgun. For example, when used on an `SSLHandler#handshakeFuture`, one would not just have to listen for the operation to succeed but *also* check that the value returned from the deferred isn't `false`. Instead, we now propagate `ClosedChannelException` as an error just like any other to make the API a bit less surprising. This allows us to remove said check from the handshake future again. Further note that other existing uses of `wrap-future` where this could happen wrap the operation in a `.closeFuture` anyway which makes this special-case unnecessary to begin with. Also included is a test for showing that putting to a closed channel sink still returns a successful deferred with a `false` value as per `manifold.stream`'s API convention.
1 parent 9f8667d commit b04da76

File tree

3 files changed

+17
-9
lines changed

3 files changed

+17
-9
lines changed

src/aleph/netty.clj

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,7 @@
232232
(d/error! d (CancellationException. "future is cancelled."))
233233

234234
(some? (.cause f))
235-
(if (instance? ClosedChannelException (.cause f))
236-
(d/success! d false)
237-
(d/error! d (.cause f)))
235+
(d/error! d (.cause f))
238236

239237
:else
240238
(d/error! d (IllegalStateException. "future in unknown state"))))

src/aleph/tcp.clj

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,8 @@
6262
(-> ssl-handler
6363
.handshakeFuture
6464
netty/wrap-future
65-
(d/on-realized (fn [ok?]
66-
;; See
67-
;; https://github.com/clj-commons/aleph/issues/618
68-
;; for why this conditional is necessary
69-
(when ok?
70-
(call-handler ctx)))
65+
(d/on-realized (fn [_]
66+
(call-handler ctx))
7167
;; No need to handle errors here since
7268
;; the SSL handler will terminate the
7369
;; whole pipeline by throwing a

test/aleph/netty_test.clj

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
(ns aleph.netty-test
2+
(:require
3+
[aleph.netty :as netty]
4+
[clojure.test :refer [deftest is]]
5+
[manifold.stream :as s])
6+
(:import
7+
(io.netty.channel.embedded EmbeddedChannel)))
8+
9+
(deftest closing-a-channel-sink
10+
(let [ch (EmbeddedChannel.)
11+
s (netty/sink ch)]
12+
(is (= true @(s/put! s "foo")))
13+
(is (nil? @(netty/wrap-future (netty/close ch))))
14+
(is (= false @(s/put! s "foo")))))

0 commit comments

Comments
 (0)