Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bencode impl now throws EOFException when appropriate, and the bencode
transport reliably throws a SocketException if an EOFException is thrown
in the course of a read attempt (which should only ever happen in
conjunction with a disconnection of the underlying socket).
  • Loading branch information
cemerick committed Oct 4, 2012
1 parent 6f6197b commit 14d6e68
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 18 deletions.
6 changes: 3 additions & 3 deletions src/main/clojure/clojure/tools/nrepl/bencode.clj
Expand Up @@ -11,7 +11,7 @@
:doc "A netstring and bencode implementation for Clojure."}
clojure.tools.nrepl.bencode
(:require [clojure.java.io :as io])
(:import (java.io IOException ByteArrayOutputStream
(:import (java.io IOException EOFException ByteArrayOutputStream
InputStream OutputStream PushbackInputStream)
clojure.lang.RT))

Expand Down Expand Up @@ -83,7 +83,7 @@
#^long [#^InputStream input]
(let [c (.read input)]
(when (neg? c)
(throw (IOException. "Invalid netstring. Unexpected end of input.")))
(throw (EOFException. "Invalid netstring. Unexpected end of input.")))
;; Here we have a quirk for example. `.read` returns -1 on end of
;; input. However the Java `Byte` has only a range from -128 to 127.
;; How does the fit together?
Expand All @@ -104,7 +104,7 @@
(let [result (.read input content offset len)]
(when (neg? result)
(throw
(IOException.
(EOFException.
"Invalid netstring. Less data available than expected.")))
(when (not= result len)
(recur (+ offset result) (- len result)))))
Expand Down
26 changes: 19 additions & 7 deletions src/main/clojure/clojure/tools/nrepl/transport.clj
Expand Up @@ -7,8 +7,8 @@
(:use [clojure.tools.nrepl.misc :only (returning uuid)])
(:refer-clojure :exclude (send))
(:import (java.io InputStream OutputStream PushbackInputStream
PushbackReader)
java.net.Socket
PushbackReader IOException EOFException)
(java.net Socket SocketException)
(java.util.concurrent SynchronousQueue LinkedBlockingQueue
BlockingQueue TimeUnit)
clojure.lang.RT))
Expand Down Expand Up @@ -73,6 +73,17 @@
(map (fn [[k v]] [k (<bytes v)]))
(into {})))

(defmacro ^{:private true} rethrow-on-disconnection
[^Socket s & body]
`(try
~@body
(catch EOFException e#
(throw (SocketException. "The transport's socket appears to have lost its connection to the nREPL server")))
(catch Throwable e#
(if (and ~s (not (.isConnected ~s)))
(throw (SocketException. "The transport's socket appears to have lost its connection to the nREPL server"))
(throw e#)))))

(defn bencode
"Returns a Transport implementation that serializes messages
over the given Socket or InputStream/OutputStream using bencode."
Expand All @@ -81,16 +92,17 @@
(let [in (PushbackInputStream. (io/input-stream in))
out (io/output-stream out)]
(fn-transport
#(let [payload (be/read-bencode in)
#(let [payload (rethrow-on-disconnection s (be/read-bencode in))
unencoded (<bytes (payload "-unencoded"))
to-decode (apply dissoc payload "-unencoded" unencoded)]
(merge (dissoc payload "-unencoded")
(when unencoded {"-unencoded" unencoded})
(<bytes to-decode)))
#(locking out
(doto out
(be/write-bencode %)
.flush))
#(rethrow-on-disconnection s
(locking out
(doto out
(be/write-bencode %)
.flush)))
(fn []
(.close in)
(.close out)
Expand Down
14 changes: 6 additions & 8 deletions src/test/clojure/clojure/tools/nrepl_test.clj
@@ -1,5 +1,5 @@
(ns clojure.tools.nrepl-test
(:import java.io.IOException)
(:import java.net.SocketException)
(:use clojure.test
clojure.tools.nrepl)
(:require (clojure.tools.nrepl [transport :as transport]
Expand Down Expand Up @@ -247,8 +247,8 @@
(defn- disconnection-exception?
[e]
; thrown? should check for the root cause!
(and (instance? IOException (root-cause e))
(re-matches #".*Unexpected end of input.*" (.getMessage (root-cause e)))))
(and (instance? SocketException (root-cause e))
(re-matches #".*lost.*connection.*" (.getMessage (root-cause e)))))

(deftest transports-fail-on-disconnects
(testing "Ensure that transports fail ASAP when the server they're connected to goes down."
Expand All @@ -267,14 +267,12 @@
(catch Throwable e
(is (disconnection-exception? e)))))

(is (thrown? IOException
(transport/recv transport)))
(is (thrown? SocketException (transport/recv transport)))
;; TODO no idea yet why two sends are *sometimes* required to get a failure
(try
(transport/send transport {"op" "eval" "code" "(+ 5 1)"})
(catch Throwable t))
(is (thrown? IOException
(transport/send transport {"op" "eval" "code" "(+ 5 1)"}))))))
(is (thrown? SocketException (transport/send transport {"op" "eval" "code" "(+ 5 1)"}))))))

(def-repl-test clients-fail-on-disconnects
(testing "Ensure that clients fail ASAP when the server they're connected to goes down."
Expand All @@ -294,7 +292,7 @@
;; TODO as noted in transports-fail-on-disconnects, *sometimes* two sends are needed
;; to trigger an exception on send to an unavailable server
(try (repl-eval session "(+ 1 1)") (catch Throwable t))
(is (thrown? IOException (repl-eval session "(+ 1 1)")))))
(is (thrown? SocketException (repl-eval session "(+ 1 1)")))))

(def-repl-test request-*in*
(is (= '((1 2 3)) (response-values (for [resp (repl-eval session "(read)")]
Expand Down

0 comments on commit 14d6e68

Please sign in to comment.