Fix wallet APIs launch command (#1574)

We used to launch a thread for API server inside the wallet crate, now we do it inside api crate, so the cmd tool launches API and exit. This fix makes sure that command will wait for API thread.

api/src/handlers.rs

@@ -805,7 +805,7 @@ pub fn start_rest_apis(
 
 	info!(LOGGER, "Starting HTTP API server at {}.", addr);
 	let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address");
-	apis.start(socket_addr, router)
+	apis.start(socket_addr, router).is_ok()
 }
 
 pub fn build_router(

api/src/rest.rs

@@ -110,13 +110,19 @@ impl ApiServer {
 	}
 
 	/// Starts the ApiServer at the provided address.
-	pub fn start(&mut self, addr: SocketAddr, router: Router) -> bool {
+	pub fn start(
+		&mut self,
+		addr: SocketAddr,
+		router: Router,
+	) -> Result<thread::JoinHandle<()>, Error> {
 		if self.shutdown_sender.is_some() {
-			error!(LOGGER, "Can't start HTTP API server, it's running already");
-			return false;
+			return Err(ErrorKind::Internal(
+				"Can't start HTTP API server, it's running already".to_string(),
+			))?;
 		}
 		let (tx, _rx) = oneshot::channel::<()>();
-		let _ = thread::Builder::new()
+		self.shutdown_sender = Some(tx);
+		thread::Builder::new()
 			.name("apis".to_string())
 			.spawn(move || {
 				let server = Server::bind(&addr)
@@ -126,21 +132,24 @@ impl ApiServer {
 					.map_err(|e| eprintln!("HTTP API server error: {}", e));
 
 				rt::run(server);
-			});
-
-		info!(LOGGER, "HTTP API server has been started");
-		self.shutdown_sender = Some(tx);
-		true
+			})
+			.map_err(|_| ErrorKind::Internal("failed to spawn API thread".to_string()).into())
 	}
 
 	/// Starts the TLS ApiServer at the provided address.
 	/// TODO support stop operation
-	pub fn start_tls(&mut self, addr: SocketAddr, router: Router, conf: TLSConfig) -> bool {
+	pub fn start_tls(
+		&mut self,
+		addr: SocketAddr,
+		router: Router,
+		conf: TLSConfig,
+	) -> Result<thread::JoinHandle<()>, Error> {
 		if self.shutdown_sender.is_some() {
-			error!(LOGGER, "Can't start HTTP API server, it's running already");
-			return false;
+			return Err(ErrorKind::Internal(
+				"Can't start HTTPS API server, it's running already".to_string(),
+			))?;
 		}
-		let _ = thread::Builder::new()
+		thread::Builder::new()
 			.name("apis".to_string())
 			.spawn(move || {
 				let cert = Identity::from_pkcs12(conf.pkcs_bytes.as_slice(), &conf.pass).unwrap();
@@ -175,10 +184,8 @@ impl ApiServer {
 					});
 
 				rt::run(server);
-			});
-
-		info!(LOGGER, "HTTPS API server has been started");
-		true
+			})
+			.map_err(|_| ErrorKind::Internal("failed to spawn API thread".to_string()).into())
 	}
 
 	/// Stops the API server, it panics in case of error

api/tests/rest.rs

@@ -69,7 +69,7 @@ fn test_start_api() {
 	router.add_middleware(counter.clone());
 	let server_addr = "127.0.0.1:14434";
 	let addr: SocketAddr = server_addr.parse().expect("unable to parse server address");
-	assert!(server.start(addr, router));
+	assert!(server.start(addr, router).is_ok());
 	let url = format!("http://{}/v1/", server_addr);
 	let index = api::client::get::<Vec<String>>(url.as_str()).unwrap();
 	assert_eq!(index.len(), 2);
@@ -94,7 +94,7 @@ fn test_start_api_tls() {
 	let router = build_router();
 	let server_addr = "127.0.0.1:14444";
 	let addr: SocketAddr = server_addr.parse().expect("unable to parse server address");
-	assert!(server.start_tls(addr, router, tls_conf));
+	assert!(server.start_tls(addr, router, tls_conf).is_ok());
 	let url = format!("https://{}/v1/", server_addr);
 	let index = api::client::get::<Vec<String>>(url.as_str()).unwrap();
 	assert_eq!(index.len(), 2);

wallet/src/libwallet/controller.rs

@@ -16,18 +16,11 @@
 //! invocations) as needed.
 //! Still experimental
 use api::{ApiServer, Handler, ResponseFuture, Router};
-use std::collections::HashMap;
-use std::marker::PhantomData;
-use std::net::SocketAddr;
-use std::sync::{Arc, Mutex};
-
+use core::core::Transaction;
+use failure::ResultExt;
 use futures::future::{err, ok};
 use futures::{Future, Stream};
 use hyper::{Body, Request, Response, StatusCode};
-use serde::{Deserialize, Serialize};
-use serde_json;
-
-use core::core::Transaction;
 use keychain::Keychain;
 use libtx::slate::Slate;
 use libwallet::api::{APIForeign, APIOwner};
@@ -35,8 +28,13 @@ use libwallet::types::{
 	CbData, OutputData, SendTXArgs, TxLogEntry, WalletBackend, WalletClient, WalletInfo,
 };
 use libwallet::{Error, ErrorKind};
+use serde::{Deserialize, Serialize};
+use serde_json;
+use std::collections::HashMap;
+use std::marker::PhantomData;
+use std::net::SocketAddr;
+use std::sync::{Arc, Mutex};
 use url::form_urlencoded;
-
 use util::secp::pedersen;
 use util::LOGGER;
 
@@ -86,8 +84,14 @@ where
 	let mut apis = ApiServer::new();
 	info!(LOGGER, "Starting HTTP Owner API server at {}.", addr);
 	let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address");
-	apis.start(socket_addr, router);
-	Ok(())
+	let api_thread = apis
+		.start(socket_addr, router)
+		.context(ErrorKind::GenericError(
+			"API thread failed to start".to_string(),
+		))?;
+	api_thread
+		.join()
+		.map_err(|e| ErrorKind::GenericError(format!("API thread paniced :{:?}", e)).into())
 }
 
 /// Listener version, providing same API but listening for requests on a
@@ -108,8 +112,15 @@ where
 	let mut apis = ApiServer::new();
 	info!(LOGGER, "Starting HTTP Foreign API server at {}.", addr);
 	let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address");
-	apis.start(socket_addr, router);
-	Ok(())
+	let api_thread = apis
+		.start(socket_addr, router)
+		.context(ErrorKind::GenericError(
+			"API thread failed to start".to_string(),
+		))?;
+
+	api_thread
+		.join()
+		.map_err(|e| ErrorKind::GenericError(format!("API thread paniced :{:?}", e)).into())
 }
 
 type WalletResponseFuture = Box<Future<Item = Response<Body>, Error = Error> + Send>;