Clean up exception handling in Peer/PeerGroup. Resolves issue 65

src/com/google/bitcoin/core/Peer.java

@@ -92,14 +92,16 @@ public class Peer {
 
     /**
      * Connects to the peer.
+     * 
+     * @throws PeerException when there is a temporary problem with the peer and we should retry later
      */
-    public void connect() {
+    public void connect() throws PeerException {
         try {
             conn = new NetworkConnection(address, params, bestHeight, 60000);
         } catch (IOException ex) {
-            throw new RuntimeException(ex);
+            throw new PeerException(ex);
         } catch (ProtocolException ex) {
-            throw new RuntimeException(ex);
+            throw new PeerException(ex);
         }
     }
 
@@ -107,13 +109,16 @@ public class Peer {
      * Runs in the peers network loop and manages communication with the peer.
      * 
      * <p>connect() must be called first
+     * 
+     * @throws PeerException when there is a temporary problem with the peer and we should retry later
      */
-    public void run() {
+    public void run() throws PeerException {
         // This should be called in the network loop thread for this peer
         if (conn == null)
             throw new RuntimeException("please call connect() first");
         
         running = true;
+        
         try {
             while (true) {
                 Message m = conn.readMessage();
@@ -130,25 +135,24 @@ public class Peer {
                     log.warn("Received unhandled message: {}", m);
                 }
             }
-        } catch (Exception e) {
+        } catch (IOException e) {
-            if (e instanceof IOException && !running) {
+            disconnect();
+            if (!running) {
                 // This exception was expected because we are tearing down the socket as part of quitting.
                 log.info("Shutting down peer loop");
             } else {
-                // We caught an unexpected exception.
+                throw new PeerException(e);
-                e.printStackTrace();
             }
+        } catch (ProtocolException e) {
+            disconnect();
+            throw new PeerException(e);
+        } catch (RuntimeException e) {
+            disconnect();
+            log.error("unexpected exception in peer loop", e);
+            throw e;
         }
 
-        try {
+        disconnect();
-            conn.shutdown();
-        } catch (IOException e) {
-            // Ignore exceptions on shutdown, socket might be dead
-        }
-        
-        synchronized (this) {
-            running = false;
-        }
     }
 
     private void processBlock(Block m) throws IOException {
@@ -383,7 +387,7 @@ public class Peer {
         if (chainHeight <= 0) {
             // This should not happen because we shouldn't have given the user a Peer that is to another client-mode
             // node. If that happens it means the user overrode us somewhere.
-            throw new RuntimeException("Peer does not have block chain");
+            return -1;
         }
         int blocksToGet = chainHeight - blockChain.getChainHead().getHeight();
         return blocksToGet;
@@ -398,6 +402,7 @@ public class Peer {
         }
         try {
             // This is the correct way to stop an IO bound loop
+            if (conn != null)
                 conn.shutdown();
         } catch (IOException e) {
             // Don't care about this.

src/com/google/bitcoin/core/PeerException.java

@@ -0,0 +1,36 @@
+/**
+ * Copyright 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.bitcoin.core;
+
+/**
+ * Thrown when a problem occurs in communicating with a peer, and we should
+ * retry. 
+ */
+public class PeerException extends Exception {
+    @SuppressWarnings("serial")
+    public PeerException(String msg) {
+        super(msg);
+    }
+
+    public PeerException(Exception e) {
+        super(e);
+    }
+
+    public PeerException(String msg, Exception e) {
+        super(msg, e);
+    }
+}

src/com/google/bitcoin/core/PeerGroup.java

@@ -25,6 +25,7 @@ import com.google.bitcoin.store.BlockStoreException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOError;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.util.Collections;
@@ -242,17 +243,13 @@ public class PeerGroup {
                                 handleNewPeer(peer);
                                 log.info("running " + peer);
                                 peer.run();
-                            } catch (RuntimeException ex) {
+                            } catch (PeerException ex) {
-                                // do not propagate RuntimeException - log and try next peer
+                                // do not propagate PeerException - log and try next peer
                                 log.error("error while talking to peer", ex);
                             } finally {
                                 // In all cases, disconnect and put the address back on the queue.
                                 // We will retry this peer after all other peers have been tried.
-                                try {
                                 peer.disconnect();
-                                } catch (RuntimeException ex) {
-                                    // ignore
-                                }
 
                                 inactives.add(address);
                                 if (peers.remove(peer))
@@ -273,7 +270,7 @@ public class PeerGroup {
                     // Fatal error
                     log.error("Block store corrupt?", e);
                     running = false;
-                    break;
+                    throw new IOError(e);
                 }
                 
                 // If we got here, we should retry this address because an error unrelated