Merge branch '285-plugins-should-throw-exceptions-for-startup-errors' into 'master'

Plugins throw exceptions for startup errors

Please review carefully as I don't know much about the code I have touched here.

Closes #285

See merge request !462

bramble-android/src/main/java/org/briarproject/bramble/plugin/droidtooth/DroidtoothPlugin.java

@@ -18,6 +18,7 @@ import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPluginCallback;
@@ -128,7 +129,7 @@ class DroidtoothPlugin implements DuplexPlugin {
 	}
 
 	@Override
-	public boolean start() throws IOException {
+	public void start() throws PluginException {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		// BluetoothAdapter.getDefaultAdapter() must be called on a thread
 		// with a message queue, so submit it to the AndroidExecutor
@@ -142,13 +143,14 @@ class DroidtoothPlugin implements DuplexPlugin {
 					}).get();
 		} catch (InterruptedException e) {
 			Thread.currentThread().interrupt();
-			throw new IOException("Interrupted while getting BluetoothAdapter");
+			LOG.warning("Interrupted while getting BluetoothAdapter");
+			throw new PluginException(e);
 		} catch (ExecutionException e) {
-			throw new IOException(e);
+			throw new PluginException(e);
 		}
 		if (adapter == null) {
 			LOG.info("Bluetooth is not supported");
-			return false;
+			throw new PluginException();
 		}
 		running = true;
 		// Listen for changes to the Bluetooth state
@@ -170,7 +172,6 @@ class DroidtoothPlugin implements DuplexPlugin {
 				LOG.info("Not enabling Bluetooth");
 			}
 		}
-		return true;
 	}
 
 	private void bind() {

bramble-android/src/main/java/org/briarproject/bramble/plugin/tcp/AndroidLanTcpPlugin.java

@@ -39,14 +39,13 @@ class AndroidLanTcpPlugin extends LanTcpPlugin {
 	}
 
 	@Override
-	public boolean start() {
+	public void start() {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		running = true;
 		// Register to receive network status events
 		networkStateReceiver = new NetworkStateReceiver();
 		IntentFilter filter = new IntentFilter(CONNECTIVITY_ACTION);
 		appContext.registerReceiver(networkStateReceiver, filter);
-		return true;
 	}
 
 	@Override

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java

@@ -25,6 +25,7 @@ import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.TorConstants;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
@@ -162,14 +163,18 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	}
 
 	@Override
-	public boolean start() throws IOException {
+	public void start() throws PluginException {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		// Install or update the assets if necessary
 		if (!assetsAreUpToDate()) installAssets();
 		LOG.info("Starting Tor");
 		// Watch for the auth cookie file being updated
-		cookieFile.getParentFile().mkdirs();
-		cookieFile.createNewFile();
+		try {
+			cookieFile.getParentFile().mkdirs();
+			cookieFile.createNewFile();
+		} catch (IOException e) {
+			throw new PluginException(e);
+		}
 		CountDownLatch latch = new CountDownLatch(1);
 		FileObserver obs = new WriteObserver(cookieFile, latch);
 		obs.startWatching();
@@ -182,8 +187,8 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		Process torProcess;
 		try {
 			torProcess = Runtime.getRuntime().exec(cmd, env, torDirectory);
-		} catch (SecurityException e) {
-			throw new IOException(e);
+		} catch (SecurityException | IOException e) {
+			throw new PluginException(e);
 		}
 		// Log the process's standard output until it detaches
 		if (LOG.isLoggable(INFO)) {
@@ -197,35 +202,39 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			if (exit != 0) {
 				if (LOG.isLoggable(WARNING))
 					LOG.warning("Tor exited with value " + exit);
-				return false;
+				throw new PluginException();
 			}
 			// Wait for the auth cookie file to be created/updated
 			if (!latch.await(COOKIE_TIMEOUT, MILLISECONDS)) {
 				LOG.warning("Auth cookie not created");
 				if (LOG.isLoggable(INFO)) listFiles(torDirectory);
-				return false;
+				throw new PluginException();
 			}
 		} catch (InterruptedException e) {
 			LOG.warning("Interrupted while starting Tor");
 			Thread.currentThread().interrupt();
-			return false;
+			throw new PluginException();
 		}
-		// Open a control connection and authenticate using the cookie file
-		controlSocket = new Socket("127.0.0.1", CONTROL_PORT);
-		controlConnection = new TorControlConnection(controlSocket);
-		controlConnection.authenticate(read(cookieFile));
-		// Tell Tor to exit when the control connection is closed
-		controlConnection.takeOwnership();
-		controlConnection.resetConf(Collections.singletonList(OWNER));
-		running = true;
-		// Register to receive events from the Tor process
-		controlConnection.setEventHandler(this);
-		controlConnection.setEvents(Arrays.asList(EVENTS));
-		// Check whether Tor has already bootstrapped
-		String phase = controlConnection.getInfo("status/bootstrap-phase");
-		if (phase != null && phase.contains("PROGRESS=100")) {
-			LOG.info("Tor has already bootstrapped");
-			connectionStatus.setBootstrapped();
+		try {
+			// Open a control connection and authenticate using the cookie file
+			controlSocket = new Socket("127.0.0.1", CONTROL_PORT);
+			controlConnection = new TorControlConnection(controlSocket);
+			controlConnection.authenticate(read(cookieFile));
+			// Tell Tor to exit when the control connection is closed
+			controlConnection.takeOwnership();
+			controlConnection.resetConf(Collections.singletonList(OWNER));
+			running = true;
+			// Register to receive events from the Tor process
+			controlConnection.setEventHandler(this);
+			controlConnection.setEvents(Arrays.asList(EVENTS));
+			// Check whether Tor has already bootstrapped
+			String phase = controlConnection.getInfo("status/bootstrap-phase");
+			if (phase != null && phase.contains("PROGRESS=100")) {
+				LOG.info("Tor has already bootstrapped");
+				connectionStatus.setBootstrapped();
+			}
+		} catch (IOException e) {
+			throw new PluginException(e);
 		}
 		// Register to receive network status events
 		networkStateReceiver = new NetworkStateReceiver();
@@ -233,7 +242,6 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		appContext.registerReceiver(networkStateReceiver, filter);
 		// Bind a server socket to receive incoming hidden service connections
 		bind();
-		return true;
 	}
 
 	private boolean assetsAreUpToDate() {
@@ -246,7 +254,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		}
 	}
 
-	private void installAssets() throws IOException {
+	private void installAssets() throws PluginException {
 		InputStream in = null;
 		OutputStream out = null;
 		try {
@@ -269,7 +277,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		} catch (IOException e) {
 			tryToClose(in);
 			tryToClose(out);
-			throw e;
+			throw new PluginException(e);
 		}
 	}
 
@@ -476,7 +484,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	}
 
 	@Override
-	public void stop() throws IOException {
+	public void stop() throws PluginException {
 		running = false;
 		tryToClose(socket);
 		if (networkStateReceiver != null)

bramble-api/src/main/java/org/briarproject/bramble/api/plugin/Plugin.java

@@ -3,7 +3,6 @@ package org.briarproject.bramble.api.plugin;
 import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 
-import java.io.IOException;
 import java.util.Collection;
 
 @NotNullByDefault
@@ -25,14 +24,14 @@ public interface Plugin {
 	int getMaxIdleTime();
 
 	/**
-	 * Starts the plugin and returns true if it started successfully.
+	 * Starts the plugin.
 	 */
-	boolean start() throws IOException;
+	void start() throws PluginException;
 
 	/**
 	 * Stops the plugin.
 	 */
-	void stop() throws IOException;
+	void stop() throws PluginException;
 
 	/**
 	 * Returns true if the plugin is running.

bramble-api/src/main/java/org/briarproject/bramble/api/plugin/PluginException.java

@@ -0,0 +1,15 @@
+package org.briarproject.bramble.api.plugin;
+
+/**
+ * An exception that indicates an error starting or stopping a {@link Plugin}.
+ */
+public class PluginException extends Exception {
+
+	public PluginException() {
+		super();
+	}
+
+	public PluginException(Throwable cause) {
+		super(cause);
+	}
+}

bramble-core/src/main/java/org/briarproject/bramble/plugin/PluginManagerImpl.java

@@ -11,6 +11,7 @@ import org.briarproject.bramble.api.plugin.ConnectionManager;
 import org.briarproject.bramble.api.plugin.Plugin;
 import org.briarproject.bramble.api.plugin.PluginCallback;
 import org.briarproject.bramble.api.plugin.PluginConfig;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.PluginManager;
 import org.briarproject.bramble.api.plugin.TransportConnectionReader;
 import org.briarproject.bramble.api.plugin.TransportConnectionWriter;
@@ -30,7 +31,6 @@ import org.briarproject.bramble.api.settings.Settings;
 import org.briarproject.bramble.api.settings.SettingsManager;
 import org.briarproject.bramble.api.ui.UiCallback;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -193,24 +193,17 @@ class PluginManagerImpl implements PluginManager, Service {
 		@Override
 		public void run() {
 			try {
-				try {
-					long start = System.currentTimeMillis();
-					boolean started = plugin.start();
-					long duration = System.currentTimeMillis() - start;
-					if (started) {
-						if (LOG.isLoggable(INFO)) {
-							LOG.info("Starting plugin " + plugin.getId()
-									+ " took " + duration + " ms");
-						}
-					} else {
-						if (LOG.isLoggable(WARNING)) {
-							LOG.warning("Plugin" + plugin.getId()
-									+ " did not start");
-						}
-					}
-				} catch (IOException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
+				long start = System.currentTimeMillis();
+				plugin.start();
+				long duration = System.currentTimeMillis() - start;
+				if (LOG.isLoggable(INFO)) {
+					LOG.info("Starting plugin " + plugin.getId() + " took " +
+							duration + " ms");
+				}
+			} catch (PluginException e) {
+				if (LOG.isLoggable(WARNING)) {
+					LOG.warning("Plugin " + plugin.getId() + " did not start");
+					LOG.log(WARNING, e.toString(), e);
 				}
 			} finally {
 				startLatch.countDown();
@@ -246,10 +239,13 @@ class PluginManagerImpl implements PluginManager, Service {
 							+ " took " + duration + " ms");
 				}
 			} catch (InterruptedException e) {
-				LOG.warning("Interrupted while waiting for plugin to start");
+				LOG.warning("Interrupted while waiting for plugin to stop");
 				// This task runs on an executor, so don't reset the interrupt
-			} catch (IOException e) {
-				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			} catch (PluginException e) {
+				if (LOG.isLoggable(WARNING)) {
+					LOG.warning("Plugin " + plugin.getId() + " did not stop");
+					LOG.log(WARNING, e.toString(), e);
+				}
 			} finally {
 				stopLatch.countDown();
 			}

bramble-core/src/main/java/org/briarproject/bramble/plugin/tcp/TcpPlugin.java

@@ -101,11 +101,10 @@ abstract class TcpPlugin implements DuplexPlugin {
 	}
 
 	@Override
-	public boolean start() {
+	public void start() {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		running = true;
 		bind();
-		return true;
 	}
 
 	protected void bind() {

bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java

@@ -9,6 +9,7 @@ import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPluginCallback;
@@ -99,7 +100,7 @@ class BluetoothPlugin implements DuplexPlugin {
 	}
 
 	@Override
-	public boolean start() throws IOException {
+	public void start() throws PluginException {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		// Initialise the Bluetooth stack
 		try {
@@ -108,13 +109,14 @@ class BluetoothPlugin implements DuplexPlugin {
 			// On Linux the user may need to install libbluetooth-dev
 			if (OsUtils.isLinux())
 				callback.showMessage("BLUETOOTH_INSTALL_LIBS");
-			return false;
+			throw new PluginException(e);
+		} catch (BluetoothStateException e) {
+			throw new PluginException(e);
 		}
 		if (LOG.isLoggable(INFO))
 			LOG.info("Local address " + localDevice.getBluetoothAddress());
 		running = true;
 		bind();
-		return true;
 	}
 
 	private void bind() {

bramble-j2se/src/main/java/org/briarproject/bramble/plugin/file/RemovableDrivePlugin.java

@@ -2,6 +2,7 @@ package org.briarproject.bramble.plugin.file;
 
 import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPluginCallback;
 
@@ -42,17 +43,24 @@ class RemovableDrivePlugin extends FilePlugin
 	}
 
 	@Override
-	public boolean start() throws IOException {
+	public void start() throws PluginException {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		running = true;
-		monitor.start(this);
-		return true;
+		try {
+			monitor.start(this);
+		} catch (IOException e) {
+			throw new PluginException(e);
+		}
 	}
 
 	@Override
-	public void stop() throws IOException {
+	public void stop() throws PluginException {
 		running = false;
-		monitor.stop();
+		try {
+			monitor.stop();
+		} catch (IOException e) {
+			throw new PluginException(e);
+		}
 	}
 
 	@Override

bramble-j2se/src/main/java/org/briarproject/bramble/plugin/modem/ModemPlugin.java

@@ -6,6 +6,7 @@ import org.briarproject.bramble.api.data.BdfList;
 import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
+import org.briarproject.bramble.api.plugin.PluginException;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.plugin.duplex.AbstractDuplexTransportConnection;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
@@ -68,7 +69,7 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 	}
 
 	@Override
-	public boolean start() {
+	public void start() throws PluginException {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		for (String portName : serialPortList.getPortNames()) {
 			if (LOG.isLoggable(INFO))
@@ -79,12 +80,12 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 				if (LOG.isLoggable(INFO))
 					LOG.info("Initialised modem on " + portName);
 				running = true;
-				return true;
+				return;
 			} catch (IOException e) {
 				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
 		}
-		return false;
+		throw new PluginException();
 	}
 
 	@Override

bramble-j2se/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java

@@ -14,7 +14,6 @@ import java.util.Map;
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 
 public class ModemPluginTest extends BrambleTestCase {
 
@@ -49,7 +48,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			oneOf(modem).start();
 			will(returnValue(true));
 		}});
-		assertTrue(plugin.start());
+		plugin.start();
 		context.assertIsSatisfied();
 	}
 
@@ -88,7 +87,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			oneOf(modem).dial(NUMBER);
 			will(returnValue(true));
 		}});
-		assertTrue(plugin.start());
+		plugin.start();
 		// A connection should be returned
 		assertNotNull(plugin.createConnection(contactId));
 		context.assertIsSatisfied();
@@ -129,7 +128,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			oneOf(modem).dial(NUMBER);
 			will(returnValue(false));
 		}});
-		assertTrue(plugin.start());
+		plugin.start();
 		// No connection should be returned
 		assertNull(plugin.createConnection(contactId));
 		context.assertIsSatisfied();
@@ -177,7 +176,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			oneOf(modem).start();
 			will(returnValue(true));
 		}});
-		assertTrue(plugin.start());
+		plugin.start();
 		// No connection should be returned
 		assertNull(plugin.createConnection(contactId));
 		context.assertIsSatisfied();