Merge #111: Retry initial client connections

26ceb4d Retry initial client connections (Elias Rohrer)

Pull request description:

  I'm not sure what of the behavior really changed since the `bitcoind` days, but here's yet another thing I stumbled across while trying to making the switch to `corepc-node`:

  Previously, `corerpc-node` would try to establish a connection via the given `Auth` cookie, and would panic if it couldn't setup a initial connection. Only the rest of the initial setup steps would be retried with time-delay.

  This however was race-y, as the auth cookie might not already be synced to disk on first startup of a fresh `bitcoind` instance (with fresh data dir), when we try to connect for the first time. Here, we move the intial connection logic into the retry loop to simply also retry it instead of panicking.

  @tcharding Seems this might be the last issue (fingers crossed) that keeps LDK and LDK Node from switching our CI over to `corepc-node`. As the switch is also blocking some features, it would be much appreciated if this could be released soonish after it's reviewed.

ACKs for top commit:
  nervana21:
    tACK 26ceb4d
  tcharding:
    utACK 26ceb4d

Tree-SHA512: f793152ba101c0646389404bfb4fcbc7cfe0c3323ea06639aa2c625c1a39bd49cae552e86e37f5ad8d6fbbf107f326fe6a410989598aaeba6c8db26dd352f267

Author: tcharding

node/src/lib.rs

@@ -383,45 +383,61 @@ impl Node {
         thread::sleep(Duration::from_millis(1000));
         assert!(process.stderr.is_none());
 
-        let mut i = 0;
+        let mut tries = 0;
         let auth = Auth::CookieFile(cookie_file.clone());
-        let client_base =
-            Client::new_with_auth(&rpc_url, auth.clone()).expect("failed to create client");
 
         let client = loop {
+            tries += 1;
+
+            if tries > 10 {
+                error!("failed to get a response from bitcoind");
+                return Err(Error::NoBitcoindInstance.into());
+            }
+
+            let client_base = match Client::new_with_auth(&rpc_url, auth.clone()) {
+                Ok(client) => client,
+                Err(e) => {
+                    error!("failed to create client: {}. Retrying!", e);
+                    thread::sleep(Duration::from_millis(1000));
+                    continue;
+                }
+            };
+
             // Just use serde value because changes to the GetBlockchainInfo type make debugging hard.
             let client_result: Result<serde_json::Value, _> =
                 client_base.call("getblockchaininfo", &[]);
 
-            if client_result.is_ok() {
-                let url = match &conf.wallet {
-                    Some(wallet) => {
-                        debug!("trying to create/load wallet: {}", wallet);
-                        // Debugging logic here implicitly tests `into_model` for create/load wallet.
-                        match client_base.create_wallet(wallet) {
-                            Ok(json) => {
-                                debug!("created wallet: {}", json.name());
-                            }
-                            Err(e) => {
-                                debug!("initial create_wallet failed, try load instead: {:?}", e);
-                                let wallet = client_base.load_wallet(wallet)?.name();
-                                debug!("loaded wallet: {}", wallet);
+            match client_result {
+                Ok(_) => {
+                    let url = match &conf.wallet {
+                        Some(wallet) => {
+                            debug!("trying to create/load wallet: {}", wallet);
+                            // Debugging logic here implicitly tests `into_model` for create/load wallet.
+                            match client_base.create_wallet(wallet) {
+                                Ok(json) => {
+                                    debug!("created wallet: {}", json.name());
+                                }
+                                Err(e) => {
+                                    debug!(
+                                        "initial create_wallet failed, try load instead: {:?}",
+                                        e
+                                    );
+                                    let wallet = client_base.load_wallet(wallet)?.name();
+                                    debug!("loaded wallet: {}", wallet);
+                                }
                             }
+                            format!("{}/wallet/{}", rpc_url, wallet)
                         }
-                        format!("{}/wallet/{}", rpc_url, wallet)
-                    }
-                    None => rpc_url,
-                };
-                debug!("creating client with url: {}", url);
-                break Client::new_with_auth(&url, auth)?;
-            }
-
-            thread::sleep(Duration::from_millis(1000));
-
-            i += 1;
-            if i > 10 {
-                error!("failed to get a response from bitcoind");
-                return Err(Error::NoBitcoindInstance.into());
+                        None => rpc_url,
+                    };
+                    debug!("creating client with url: {}", url);
+                    break Client::new_with_auth(&url, auth)?;
+                }
+                Err(e) => {
+                    error!("failed to get a response from bitcoind: {}. Retrying!", e);
+                    thread::sleep(Duration::from_millis(1000));
+                    continue;
+                }
             }
         };