[187218] Fix error reporting for connect()

rse/plugins/org.eclipse.rse.connectorservice.telnet/src/org/eclipse/rse/internal/connectorservice/telnet/TelnetConnectorService.java

@@ -13,14 +13,13 @@
  * Martin Oberhuber (Wind River) - [178606] fix endless loop in readUntil()
  * Sheldon D'souza (Celunite) - [186536] login and password should be configurable
  * Sheldon D'souza (Celunite) - [186570] handle invalid user id and password more gracefully
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  *******************************************************************************/
 package org.eclipse.rse.internal.connectorservice.telnet;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.lang.reflect.InvocationTargetException;
-import java.net.SocketException;
 
 import org.apache.commons.net.telnet.TelnetClient;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -59,7 +58,8 @@ public class TelnetConnectorService extends StandardConnectorService implements
 	public static final String PROPERTY_PASSWORD_PROMPT = "Password.Prompt"; //$NON-NLS-1$
 	public static final String PROPERTY_COMMAND_PROMPT = "Command.Prompt"; //$NON-NLS-1$
 
-	private static final int TELNET_DEFAULT_PORT = 23;
+	private static final int TELNET_DEFAULT_PORT = 23; // TODO Make configurable
+	private static final int TELNET_CONNECT_TIMEOUT = 10; //seconds - TODO: Make configurable
 	private TelnetClient fTelnetClient = new TelnetClient();
 	private SessionLostHandler fSessionLostHandler;
 	private InputStream in;
@@ -68,6 +68,7 @@ public class TelnetConnectorService extends StandardConnectorService implements
 	private static final int ERROR_CODE = 100; // filed error code
 	private static final int SUCCESS_CODE = 150; // login pass code
 	private static final int CONNECT_CLOSED = 200; // code for end of login attempts
+	private static final int CONNECT_CANCELED = 250; // code for cancel progress
 
 	public TelnetConnectorService(IHost host) {
 		super(TelnetConnectorResources.TelnetConnectorService_Name,
@@ -111,7 +112,8 @@ public class TelnetConnectorService extends StandardConnectorService implements
 		String host = getHostName();
 		String user = getUserId();
 		String password = ""; //$NON-NLS-1$
-
+		int status = ERROR_CODE;
+		Exception nestedException = null;
 		try {
 			Activator.trace("Telnet Service: Connecting....."); //$NON-NLS-1$
 			fTelnetClient.connect(host, TELNET_DEFAULT_PORT);
@@ -123,31 +125,58 @@ public class TelnetConnectorService extends StandardConnectorService implements
 			in = fTelnetClient.getInputStream();
 			out = new PrintStream(fTelnetClient.getOutputStream());
 
+			long millisToEnd = System.currentTimeMillis() + TELNET_CONNECT_TIMEOUT*1000;
 			LoginThread checkLogin = new LoginThread(user, password);
 			checkLogin.start();
+			while (checkLogin.isAlive() && System.currentTimeMillis()<millisToEnd) {
+				if (monitor!=null) {
+					monitor.worked(1);
+					if (monitor.isCanceled()) {
+						status = CONNECT_CANCELED;
+						//Thread will be interrupted by sessionDisconnect()
+						//checkLogin.interrupt();
+						break;
+					}
+				}
+				Display d = Display.getCurrent();
+				if (d!=null) {
+					while(d.readAndDispatch()) {
+						//get next event if on dispatch thread
+					}
+				}
+				checkLogin.join(500);
+			}
+			if (status != CONNECT_CANCELED) {
+				status = checkLogin.getLoginStatus();
 				checkLogin.join();
-			int status = checkLogin.getLoginStatus();
-			if (status != SUCCESS_CODE) {
-				SystemMessage msg = RSEUIPlugin
-						.getPluginMessage(ISystemMessages.MSG_COMM_CONNECT_FAILED);
-				msg.makeSubstitution(getHost().getAliasName());
-				internalDisconnect(null);
-				throw new SystemMessageException(msg);
-
 			}
-
+		} catch (Exception e) {
-			Activator.trace("Telnet Service: Connected"); //$NON-NLS-1$
+			Activator.trace("Telnet Service failed: " + e.toString()); //$NON-NLS-1$
-		} catch (SocketException se) {
+			nestedException = e;
-			Activator.trace("Telnet Service failed: " + se.toString()); //$NON-NLS-1$
+		} finally {
-			sessionDisconnect();
+			if (status == CONNECT_CANCELED) {
-		} catch (IOException ioe) {
+				Activator.trace("Telnet Service: Canceled"); //$NON-NLS-1$
-			Activator.trace("Telnet Service failed: " + ioe.toString()); //$NON-NLS-1$
+				sessionDisconnect(); //will eventually destroy the LoginThread
-			sessionDisconnect();
+			} else if (status == SUCCESS_CODE) {
-		}
-
 				fSessionLostHandler = new SessionLostHandler(this);
 				notifyConnection();
-
+				Activator.trace("Telnet Service: Connected"); //$NON-NLS-1$
+			} else {
+				Activator.trace("Telnet Service: Connect failed"); //$NON-NLS-1$
+				//TODO pass the nested exception as well as original prompts
+				//from the remote side with the SystemMessageException for user diagnostics
+				SystemMessage msg;
+				if (nestedException!=null) {
+					msg = RSEUIPlugin.getPluginMessage(ISystemMessages.MSG_EXCEPTION_OCCURRED);
+					msg.makeSubstitution(nestedException);
+				} else {
+					msg = RSEUIPlugin.getPluginMessage(ISystemMessages.MSG_COMM_AUTH_FAILED);
+					msg.makeSubstitution(getHost().getAliasName());
+				}
+				internalDisconnect(null);
+				throw new SystemMessageException(msg);
+			}
+		}
 	}
 
 	/**

rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/subsystems/IConnectorService.java

@@ -14,6 +14,7 @@
  * David Dykstal (IBM) - 168977: refactoring IConnectorService
  * Martin Oberhuber (Wind River) - [175262] IHost.getSystemType() should return IRSESystemType
  * Martin Oberhuber (Wind River) - [185750] Remove IConnectorService.getHostType() 
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.core.subsystems;
@@ -21,6 +22,7 @@ package org.eclipse.rse.core.subsystems;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.rse.core.model.IHost;
 import org.eclipse.rse.core.model.IRSEModelObject;
+import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 
 /**
  * A connector service provides the means of establishing a connection from 
@@ -79,6 +81,7 @@ public interface IConnectorService extends IRSEModelObject {
 	 * @param monitor a monitor for tracking the progress and canceling a connect
 	 * operation.
 	 * @throws Exception an exception of there is a failure to connect.
+	 *    Typically, this will be a {@link SystemMessageException}.
 	 */
 	public void connect(IProgressMonitor monitor) throws Exception;
 
@@ -87,6 +90,7 @@ public interface IConnectorService extends IRSEModelObject {
 	 * @param monitor a monitor for tracking the progress and canceling a disconnect
 	 * operation.
 	 * @throws Exception an exception of the disconnect fails.
+	 *    Typically, this will be a {@link SystemMessageException}.
 	 */
 	public void disconnect(IProgressMonitor monitor) throws Exception;
 

rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/subsystems/ISubSystem.java

@@ -13,6 +13,7 @@
  * Contributors:
  * Martin Oberhuber (Wind River) - [182454] improve getAbsoluteName() documentation
  * Martin Oberhuber (Wind River) - [186128] Move IProgressMonitor last in all API
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.core.subsystems;
@@ -357,9 +358,13 @@ public interface ISubSystem extends ISystemFilterPoolReferenceManagerProvider, I
 	public boolean isOffline();
 
 	/**
-	 * Connect to the remote system. Does not force a prompt for a password.
+	 * Connect to the remote system.
+	 * Does not force a prompt for a password. Works from UI or non-UI thread.
 	 * 
-	 * @deprecated use {@link #connect(IProgressMonitor, boolean)}
+	 * @deprecated use {@link #connect(IProgressMonitor, boolean)} from background
+	 *    thread, or {@link #connect(boolean, IRSECallback) from UI thread.
+	 * @throws Exception an exception if there is a failure to connect.
+	 *    Typically, this will be a {@link SystemMessageException}.
 	 */
 	public void connect() throws Exception;
 	
@@ -377,20 +382,29 @@ public interface ISubSystem extends ISystemFilterPoolReferenceManagerProvider, I
 	 * @param monitor the progress monitor. Must not be <code>null</code>.
 	 * @param forcePrompt forces the prompt dialog to be displayed
 	 *     even if the password is currently in memory.
+	 * @throws Exception an exception if there is a failure to connect.
+	 *    Typically, this will be a {@link SystemMessageException}.
 	 */
 	public void connect(IProgressMonitor monitor, boolean forcePrompt) throws Exception;
 
 	/**
-	 * Asynchronously connect to the remote system.
+	 * Asynchronously connect to the remote system, optionally forcing a signon prompt 
-	 * 
+	 * even if the password is cached in memory or on disk.
-	 * An Eclipse background job with a progress monitor will 
+	 * <p/> 
-	 * be created automatically. If the optional callback is 
+	 * This method must be called on the UI Thread! An Eclipse background job with a 
-	 * given, will be called when the connect is complete.
+	 * progress monitor will be created automatically. If the optional callback is 
+	 * given, it will be called when the connect is complete.
+	 * You do not need to override this, as it does the progress monitor reporting 
+	 * for you.
+	 * <p/>
+	 * Override internalConnect if you want, but by default it calls 
+	 * <code>getConnectorService().connect(IProgressMonitor)</code>.
 	 * 
 	 * @param forcePrompt forces the prompt dialog even if the password is in mem
 	 * @param callback to call after connect is complete.
 	 *     May be <code>null</code>.
-	 * @throws Exception
+	 * @throws Exception an exception if there is a failure to connect.
+	 *    Typically, this will be a {@link SystemMessageException}.
 	 */
 	public void connect(boolean forcePrompt, IRSECallback callback) throws Exception;
 

rse/plugins/org.eclipse.rse.services.telnet/src/org/eclipse/rse/internal/services/telnet/shell/TelnetShellWriterThread.java

@@ -8,11 +8,16 @@
  * Contributors: 
  * Martin Oberhuber (Wind River) - initial API and implementation 
  * Sheldon D'souza (Celunite) - Adapted from SshShellWriterThread
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  *******************************************************************************/
 package org.eclipse.rse.internal.services.telnet.shell;
 
 import java.io.PrintWriter;
 
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.rse.internal.services.telnet.Activator;
+
 public class TelnetShellWriterThread extends Thread {
 	
 	private PrintWriter fOutputWriter;
@@ -90,6 +95,11 @@ public class TelnetShellWriterThread extends Thread {
 			}
 		} catch(InterruptedException e) {
 			/* no special handling -> close stream */
+		} catch(Exception e) {
+			Activator.getDefault().getLog().log(new Status(IStatus.WARNING,
+					Activator.PLUGIN_ID,
+					e.getLocalizedMessage()!=null ? e.getLocalizedMessage() : e.getClass().getName(),
+					e));
 		} finally {
 			stopThread();
 //			if( fOutputWriter != null )

rse/plugins/org.eclipse.rse.shells.ui/src/org/eclipse/rse/internal/shells/ui/actions/SystemCommandAction.java

@@ -15,11 +15,11 @@
  * Martin Oberhuber (Wind River) - [186128] Move IProgressMonitor last in all API
  * Martin Oberhuber (Wind River) - [174945] Remove obsolete icons from rse.shells.ui
  * Martin Oberhuber (Wind River) - [186640] Add IRSESystemType.testProperty() 
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.internal.shells.ui.actions;
 
-import java.lang.reflect.InvocationTargetException;
 import java.util.Iterator;
 
 import org.eclipse.core.runtime.IAdaptable;
@@ -43,6 +43,7 @@ import org.eclipse.rse.internal.shells.ui.view.SystemCommandsUI;
 import org.eclipse.rse.internal.shells.ui.view.SystemCommandsViewPart;
 import org.eclipse.rse.services.clientserver.PathUtility;
 import org.eclipse.rse.services.clientserver.messages.SystemMessage;
+import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 import org.eclipse.rse.shells.ui.RemoteCommandHelpers;
 import org.eclipse.rse.subsystems.files.core.subsystems.IRemoteFile;
 import org.eclipse.rse.subsystems.shells.core.subsystems.IRemoteCmdSubSystem;
@@ -50,8 +51,10 @@ import org.eclipse.rse.subsystems.shells.core.subsystems.IRemoteCommandShell;
 import org.eclipse.rse.ui.ISystemIconConstants;
 import org.eclipse.rse.ui.ISystemMessages;
 import org.eclipse.rse.ui.RSEUIPlugin;
+import org.eclipse.rse.ui.SystemBasePlugin;
 import org.eclipse.rse.ui.actions.SystemBaseAction;
 import org.eclipse.rse.ui.dialogs.SystemPromptDialog;
+import org.eclipse.rse.ui.messages.SystemMessageDialog;
 import org.eclipse.rse.ui.view.ISystemViewElementAdapter;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.ModifyEvent;
@@ -112,9 +115,13 @@ public class SystemCommandAction extends SystemBaseAction
 				IRemoteCommandShell cmd = _ss.runShell(_selected, monitor);
 				Display.getDefault().asyncExec(new UpdateOutputRunnable(_cmdsPart, cmd));
 			}
-			catch (Exception e)
+			catch (SystemMessageException e) {
-			{
+				SystemMessageDialog.displayMessage(e);
-				
+			}
+			catch (Exception e) {
+				SystemBasePlugin.logError(
+						e.getLocalizedMessage()!=null ? e.getLocalizedMessage() : e.getClass().getName(),
+						e);
 			}
 			return Status.OK_STATUS;
 		}
@@ -594,24 +601,14 @@ public class SystemCommandAction extends SystemBaseAction
 
 	}
 	
-	private boolean connect(SubSystem ss, IProgressMonitor monitor)
+	private boolean connect(SubSystem ss, IProgressMonitor monitor) throws Exception
 	{
 		if (!ss.isConnected())
 		{
 			
 			Display dis = Display.getDefault();
 			dis.syncExec(new PromptForPassword(ss));
-			try
-			{
 			ss.getConnectorService().connect(monitor);
-			}
-			catch (InvocationTargetException exc)
-			{
-			}
-			catch (Exception e)
-			{
-			}
-			
 			dis.asyncExec(new UpdateRegistry(ss));			
 		}
 		return true;

rse/plugins/org.eclipse.rse.ui/UI/org/eclipse/rse/internal/ui/actions/SystemConnectAction.java

@@ -13,6 +13,7 @@
  * Contributors:
  * Martin Oberhuber (Wind River) - [175262] IHost.getSystemType() should return IRSESystemType 
  * Martin Oberhuber (Wind River) - [186640] Add IRSESystemType.testProperty()
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.internal.ui.actions;
@@ -23,9 +24,12 @@ import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.rse.core.subsystems.ISubSystem;
 import org.eclipse.rse.internal.ui.SystemResources;
+import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 import org.eclipse.rse.ui.ISystemContextMenuConstants;
 import org.eclipse.rse.ui.RSEUIPlugin;
+import org.eclipse.rse.ui.SystemBasePlugin;
 import org.eclipse.rse.ui.actions.SystemBaseAction;
+import org.eclipse.rse.ui.messages.SystemMessageDialog;
 import org.eclipse.swt.widgets.Shell;
 
 /**
@@ -50,7 +54,15 @@ public class SystemConnectAction extends SystemBaseAction
 					_subsystem.connect(monitor, false);
 				else	
 				  	_subsystem.connect(monitor, true);
-			} catch (Exception exc) {} // msg already shown
+			}
+			catch (SystemMessageException e) {
+				SystemMessageDialog.displayMessage(e);
+			}
+			catch (Exception e) {
+				SystemBasePlugin.logError(
+						e.getLocalizedMessage()!=null ? e.getLocalizedMessage() : e.getClass().getName(),
+						e);
+			}
 			if (monitor.isCanceled())
 			{
 				return Status.CANCEL_STATUS;

rse/plugins/org.eclipse.rse.ui/UI/org/eclipse/rse/internal/ui/actions/SystemConnectAllSubSystemsAction.java

@@ -12,6 +12,7 @@
  * 
  * Contributors:
  * Martin Oberhuber (Wind River) - [186773] split ISystemRegistryUI from ISystemRegistry
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.internal.ui.actions;
@@ -21,6 +22,7 @@ import java.util.List;
 
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.rse.core.RSECorePlugin;
@@ -29,8 +31,11 @@ import org.eclipse.rse.core.model.ISystemRegistry;
 import org.eclipse.rse.core.subsystems.IConnectorService;
 import org.eclipse.rse.core.subsystems.ISubSystem;
 import org.eclipse.rse.internal.ui.SystemResources;
+import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 import org.eclipse.rse.ui.ISystemContextMenuConstants;
+import org.eclipse.rse.ui.SystemBasePlugin;
 import org.eclipse.rse.ui.actions.SystemBaseAction;
+import org.eclipse.rse.ui.messages.SystemMessageDialog;
 import org.eclipse.swt.widgets.Shell;
 
 /**
@@ -63,17 +68,23 @@ public class SystemConnectAllSubSystemsAction extends SystemBaseAction
 			            {
 			                subsystem.connect(monitor, false);
 			            }
-			            catch (Exception e) 
+						catch (SystemMessageException e) {
-			            {	
+							//TODO should we collect all messages and just show one dialog with a MultiStatus?
 			                failedSystems.add(system);
-			                
+							SystemMessageDialog.displayMessage(e);
+						}
+						catch (Exception e) {
+			                failedSystems.add(system);
+			                if ((e instanceof InterruptedException) || (e instanceof OperationCanceledException)) {
 				                // if the user was prompted for password and cancelled
 				                // or if the connect was interrupted for some other reason
 				                // we don't attempt to connect the other subsystems
-			                if (e instanceof InterruptedException) {
 								break;
 			                }
-			            }// msg already shown	
+							SystemBasePlugin.logError(
+									e.getLocalizedMessage()!=null ? e.getLocalizedMessage() : e.getClass().getName(),
+									e);
+						}
 			        }
 			    }
 			} 

rse/plugins/org.eclipse.rse.ui/subsystems/org/eclipse/rse/core/subsystems/SubSystem.java

@@ -22,6 +22,7 @@
  * Martin Oberhuber (Wind River) - [186128] Move IProgressMonitor last in all API
  * Martin Oberhuber (Wind River) - [186640] Add IRSESystemType.testProperty() 
  * Martin Oberhuber (Wind River) - [186773] split ISystemRegistryUI from ISystemRegistry
+ * Martin Oberhuber (Wind River) - [187218] Fix error reporting for connect() 
  ********************************************************************************/
 
 package org.eclipse.rse.core.subsystems;
@@ -1556,32 +1557,6 @@ public abstract class SubSystem extends RSEModelObject
     	}
     }
 
-	/**
-	 * Use this job to connect from a non-UI thread. Since this extends UIJob it will
-	 * run on the UI thread when scheduled.
-	 */
-	private class ConnectFromBackgroundJob extends UIJob {
-		public ConnectFromBackgroundJob() {
-			super(GenericMessages.RSESubSystemOperation_Connect_message);
-		}
-		public IStatus runInUIThread(IProgressMonitor monitor) {
-			IStatus result = Status.OK_STATUS;
-			try {
-				connect(monitor, false);
-			} catch (InterruptedException e) {
-				result = Status.CANCEL_STATUS;
-			} catch (Exception e) {
-				e.printStackTrace();
-				String excMsg = e.getMessage();
-				if ((excMsg == null) || (excMsg.length() == 0)) excMsg = "Exception " + e.getClass().getName(); //$NON-NLS-1$
-				SystemMessage sysMsg = RSEUIPlugin.getPluginMessage(ISystemMessages.MSG_CONNECT_FAILED);
-				sysMsg.makeSubstitution(getHostName(), excMsg);
-				result = new Status(IStatus.ERROR, RSEUIPlugin.PLUGIN_ID, IStatus.OK, sysMsg.getLevelOneText(), e);
-			}
-			return result;
-		}
-	}
-	
     /**
      * Represents the subsystem operation of connecting the subsystem to the remote machine.
      */
@@ -1597,6 +1572,15 @@ public abstract class SubSystem extends RSEModelObject
     		_callback = callback;
     	}
     	
+    	public IStatus run(IProgressMonitor monitor) {
+    		IStatus status = super.run(monitor);
+			if (_callback != null)
+			{
+				_callback.done(status, null);
+			}
+			return status;
+    	}
+    	
     	public void performOperation(IProgressMonitor mon) throws InterruptedException, Exception
     	{
     		String msg = null;
@@ -1610,16 +1594,6 @@ public abstract class SubSystem extends RSEModelObject
 
     	    ISystemRegistry registry = RSECorePlugin.getTheSystemRegistry();
 			registry.connectedStatusChange(_ss, true, false);
-			
-			if (_callback != null)
-			{
-				IStatus status = Status.OK_STATUS;
-				if (!isConnected())
-				{
-					status = Status.CANCEL_STATUS;
-				}
-				_callback.done(status, null);
-			}
     	}
     }
 
@@ -2250,29 +2224,17 @@ public abstract class SubSystem extends RSEModelObject
 	public void connect() throws Exception {
 		if (!isConnected()) {
 			if (Display.getCurrent() == null) {
-				ConnectFromBackgroundJob job = new ConnectFromBackgroundJob();
+				connect(new NullProgressMonitor(), false);
-				job.setPriority(Job.INTERACTIVE);
-				job.schedule();
-				job.join();
 			} else {
 				connect(false, null);
 			}
-//			Display display = Display.getCurrent();
-//			while (job.getResult() == null) {
-//				while (display != null && display.readAndDispatch()) {
-//					//Process everything on event queue
-//				}
-//				if (job.getResult() == null) Thread.sleep(200);
-//			}
 		}
 	}
 
-	/**
+	/*
 	 * Connect to a remote system with a monitor.
 	 * Required for Bug 176603
-	 *
+	 * @see org.eclipse.rse.core.subsystems.ISubSystem#connect(org.eclipse.core.runtime.IProgressMonitor, boolean)
-	 * @param monitor the progress monitor
-	 * @param forcePrompt indicates whether to prompt even if password is in memory
 	 */
 	public void connect(IProgressMonitor monitor, boolean forcePrompt) throws Exception 
 	{				
@@ -2297,12 +2259,10 @@ public abstract class SubSystem extends RSEModelObject
 					}
 				}
 			});
-			
 			try {
+				if (exception[0]!=null)
+					throw exception[0];
 				getConnectorService().connect(monitor);
-			} catch(Exception e) {
-				e.printStackTrace();
-			}
 				if (isConnected()) {
 					final SubSystem ss = this;
 					//Notify connect status change
@@ -2312,20 +2272,15 @@ public abstract class SubSystem extends RSEModelObject
 						}
 					});
 				}
+			} finally {
 				monitor.done();
 			}
 		}
+	}
 	
-	/**
+	/*
-	 * Connect to the remote system, optionally forcing a signon prompt even if the password
+	 * (non-Javadoc)
-	 * is cached in memory or on disk.
+	 * @see org.eclipse.rse.core.subsystems.ISubSystem#connect(boolean, org.eclipse.rse.core.model.IRSECallback)
-	 * You do not need to override this, as it does the progress monitor and error message
-	 * displaying for you.
-	 * <p>
-	 * Override internalConnect if you want, but by default it calls getSystem().connect(IProgressMonitor).
-	 * 
-	 * @param forcePrompt Forces the signon prompt to be displayed even if a valid password in cached in memory
-	 * or saved on disk.
 	 */
 	public void connect(boolean forcePrompt, IRSECallback callback) throws Exception {
 		// yantzi: artemis60, (defect 53082) check that the connection has not been deleted before continuing,
@@ -2354,6 +2309,7 @@ public abstract class SubSystem extends RSEModelObject
 //dwd				((ProgressMonitorDialog) runnableContext).setCancelable(true);
 //dwd			}
 			getConnectorService().acquireCredentials(forcePrompt); // prompt for userid and password
+			//FIXME Error reporting from the ConnectJob? How is the exception thrown?
 			ConnectJob job = new ConnectJob(this, callback);
 			scheduleJob(job, null);
 		}
@@ -2839,13 +2795,12 @@ public abstract class SubSystem extends RSEModelObject
     	return null;
     }
 
-
-
-
 	 /**
 	 * Get the progress monitor dialog for this operation. We try to 
 	 *  use one for all phases of a single operation, such as connecting
 	 *  and resolving.
+	 * @deprecated this is scheduled to be removed since we want to 
+	 *     avoid UI components in SubSystem.
 	 */
 	protected IRunnableContext getRunnableContext(/*Shell rshell*/) {
 		if (Display.getCurrent() == null) {