[233993] Improve EFS error reporting

rse/plugins/org.eclipse.rse.efs/src/org/eclipse/rse/internal/efs/RSEFileStoreImpl.java

@@ -28,6 +28,7 @@
  * Kevin Doyle 		(IBM)		 - [210673] [efs][nls] Externalize Strings in RSEFileStore and RSEFileStoreImpl
  * Timur Shipilov   (Xored)      - [224540] [efs] RSEFileStore.mkdir(EFS.NONE, null) doesn't create parent folder
  * David Dykstal (IBM) [230821] fix IRemoteFileSubSystem API to be consistent with IFileService
+ * Martin Oberhuber (Wind River) - [233993] Improve EFS error reporting
  ********************************************************************************/
 
 package org.eclipse.rse.internal.efs;
@@ -52,6 +53,7 @@ import org.eclipse.rse.core.RSECorePlugin;
 import org.eclipse.rse.core.model.IHost;
 import org.eclipse.rse.core.model.ISystemRegistry;
 import org.eclipse.rse.core.subsystems.RemoteChildrenContentsType;
+import org.eclipse.rse.services.clientserver.messages.SystemElementNotFoundException;
 import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 import org.eclipse.rse.services.files.IFileService;
 import org.eclipse.rse.services.files.IHostFile;
@@ -179,7 +181,7 @@ public class RSEFileStoreImpl extends FileStore
 		return unconnected;
 	}
 
-	
+
 	/**
 	 * Return the best available remote file subsystem for a connection.
 	 * Criteria are:
@@ -189,7 +191,7 @@ public class RSEFileStoreImpl extends FileStore
 	 * <li>An unconnected FileServiceSubsystem</li>
 	 * <li>An unconnected IRemoteFileSubSystem</li>
 	 * </ol>
-	 * 
+	 *
 	 * @param host the connection to check
 	 * @return an IRemoteFileSubSystem for the given connection, or
 	 *         <code>null</code> if no IRemoteFileSubSystem is configured.
@@ -556,6 +558,24 @@ public class RSEFileStoreImpl extends FileStore
     	return exceptionText;
     }
 
+	/**
+	 * Re-interpret RSE internal exceptions into proper EFS CoreException.
+	 *
+	 * @param e Original exception from RSE SubSystems
+	 * @param codeHint hint as to what the EFS Error Code might be
+	 * @throws CoreException create CoreException according to EFS specification
+	 */
+	private void rethrowCoreException(Exception e, int codeHint) throws CoreException {
+		//default pluginId to the EFS provider; override by root if possible
+		String pluginId = Activator.getDefault().getBundle().getSymbolicName();
+		String msg = getExceptionMessage(toString(), e);
+		int code = codeHint;
+		if (e instanceof SystemElementNotFoundException) {
+			code = EFS.ERROR_NOT_EXISTS;
+		}
+		throw new CoreException(new Status(IStatus.ERROR, pluginId, code, msg, e));
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.eclipse.core.filesystem.provider.FileStore#putInfo(org.eclipse.core.filesystem.IFileInfo, int, org.eclipse.core.runtime.IProgressMonitor)
@@ -566,16 +586,14 @@ public class RSEFileStoreImpl extends FileStore
 		IRemoteFileSubSystem subSys = remoteFile.getParentRemoteFileSubSystem();
 		try {
 			if ((options & EFS.SET_ATTRIBUTES) != 0) {
-				//We cannot currently write isExecutable(), isHidden()
+				// We cannot currently write isExecutable(), isHidden()
 				subSys.setReadOnly(remoteFile, info.getAttribute(EFS.ATTRIBUTE_READ_ONLY), monitor);
 			}
 			if ((options & EFS.SET_LAST_MODIFIED) != 0) {
 				subSys.setLastModified(remoteFile, info.getLastModified(), monitor);
 			}
-		} catch(Exception e) {
+		} catch (Exception e) {
-			throw new CoreException(new Status(IStatus.ERROR,
+			rethrowCoreException(e, EFS.ERROR_WRITE);
-					Activator.getDefault().getBundle().getSymbolicName(),
-					getExceptionMessage(toString(), e), e));
 		}
 	}
 
@@ -591,7 +609,8 @@ public class RSEFileStoreImpl extends FileStore
 		if (remoteFile.isDirectory()) {
 			throw new CoreException(new Status(IStatus.ERROR,
 					Activator.getDefault().getBundle().getSymbolicName(),
-					Messages.CANNOT_OPEN_STREAM_ON_FOLDER));
+					EFS.ERROR_WRONG_TYPE,
+					Messages.CANNOT_OPEN_STREAM_ON_FOLDER, null));
 		}
 
 		if (remoteFile.isFile()) {
@@ -600,13 +619,15 @@ public class RSEFileStoreImpl extends FileStore
 			}
 			catch (SystemMessageException e) {
 				cacheRemoteFile(null);
-				throw new CoreException(new Status(IStatus.ERROR,
+				rethrowCoreException(e, EFS.ERROR_READ);
-						Activator.getDefault().getBundle().getSymbolicName(),
-						getExceptionMessage(null, e), e));
 			}
 		}
 
-		return null;
+		//file does not exist, apparently
+		//TODO use Java MessageFormat for embedding filename in message
+		throw new CoreException(new Status(IStatus.ERROR, Activator.getDefault().getBundle().getSymbolicName(),
+		    // EFS.ERROR_NOT_EXISTS,
+			EFS.ERROR_READ, Messages.FILE_STORE_DOES_NOT_EXIST + ": " + toString(), null)); //$NON-NLS-1$
 	}
 
 	/*
@@ -615,37 +636,49 @@ public class RSEFileStoreImpl extends FileStore
 	 */
 	public IFileStore mkdir(int options, IProgressMonitor monitor) throws CoreException
 	{
-		if (options == EFS.NONE) {
+		//TODO Check should be done by IRemoteFileSubSystem.createFolders()
-			IFileStore parent = getParent();
+		//if ((options & EFS.SHALLOW)!=0) {
-			if (parent != null) {
+		//	IFileStore parent = getParent();
-				parent.mkdir(options, monitor);
+		//	if (parent == null) || !parent.{
-			}
+		//		parent.mkdir(options, monitor);
-		}
+		//	}
+		//}
 
 		cacheRemoteFile(null);
 		IRemoteFile remoteFile = getRemoteFileObject(monitor, false);
 		if (remoteFile==null) {
 			throw new CoreException(new Status(IStatus.ERROR,
 					Activator.getDefault().getBundle().getSymbolicName(),
-					Messages.COULD_NOT_GET_REMOTE_FILE));
+					EFS.ERROR_NOT_EXISTS,
+					Messages.COULD_NOT_GET_REMOTE_FILE, null));
 		}
 		IRemoteFileSubSystem subSys = remoteFile.getParentRemoteFileSubSystem();
 		if (!remoteFile.exists()) {
 			try {
-				remoteFile = subSys.createFolder(remoteFile, monitor);
+				if ((options & EFS.SHALLOW) != 0) {
+					//TODO following check should be obsolete
+					if (!remoteFile.getParentRemoteFile().exists()) {
+						throw new CoreException(new Status(IStatus.ERROR,
+								Activator.getDefault().getBundle().getSymbolicName(),
+								EFS.ERROR_WRITE,
+								Messages.FILE_STORE_DOES_NOT_EXIST, null));
+					}
+					remoteFile = subSys.createFolder(remoteFile, monitor);
+				} else {
+					remoteFile = subSys.createFolders(remoteFile, monitor);
+				}
 				cacheRemoteFile(remoteFile);
 			}
 			catch (SystemMessageException e) {
-				throw new CoreException(new Status(IStatus.ERROR,
+				rethrowCoreException(e, EFS.ERROR_WRITE);
-						Activator.getDefault().getBundle().getSymbolicName(),
-						getExceptionMessage(null, e), e));
 			}
 			return _store;
 		}
 		else if (remoteFile.isFile()) {
 			throw new CoreException(new Status(IStatus.ERROR,
 					Activator.getDefault().getBundle().getSymbolicName(),
-					Messages.FILE_NAME_EXISTS));
+					EFS.ERROR_WRONG_TYPE,
+					Messages.FILE_NAME_EXISTS, null));
 		}
 		else {
 			return _store;
@@ -671,9 +704,7 @@ public class RSEFileStoreImpl extends FileStore
 				cacheRemoteFile(remoteFile);
 			}
 			catch (SystemMessageException e) {
-				throw new CoreException(new Status(IStatus.ERROR,
+				rethrowCoreException(e, EFS.ERROR_WRITE);
-						Activator.getDefault().getBundle().getSymbolicName(),
-						getExceptionMessage(null, e), e));
 			}
 		}
 
@@ -688,20 +719,18 @@ public class RSEFileStoreImpl extends FileStore
 				return subSys.getOutputStream(remoteFile.getParentPath(), remoteFile.getName(), options, monitor);
 			}
 			catch (SystemMessageException e) {
-				throw new CoreException(new Status(IStatus.ERROR,
+				rethrowCoreException(e, EFS.ERROR_WRITE);
-						Activator.getDefault().getBundle().getSymbolicName(),
-						Messages.CANNOT_OPEN_STREAM_ON_FOLDER, e));
 			}
 		}
 		else if (remoteFile.isDirectory()) {
 			throw new CoreException(new Status(IStatus.ERROR,
 					Activator.getDefault().getBundle().getSymbolicName(),
-					Messages.CANNOT_OPEN_STREAM_ON_FOLDER));
+					EFS.ERROR_WRONG_TYPE,
-		}
+					Messages.CANNOT_OPEN_STREAM_ON_FOLDER, null));
-		else {
-			//TODO check what to do for symbolic links and other strange stuff
-			return null;
 		}
+		//Fallback: No file, no folder?
+		//TODO check what to do for symbolic links and other strange stuff
+		return null;
 	}
 
 	/*
@@ -716,10 +745,11 @@ public class RSEFileStoreImpl extends FileStore
 			cacheRemoteFile(null);
 			subSys.delete(remoteFile, monitor);
 		}
+		catch (SystemElementNotFoundException e) {
+			/* not considered an error by EFS -- ignore */
+		}
 		catch (SystemMessageException e) {
-			throw new CoreException(new Status(IStatus.ERROR,
+			rethrowCoreException(e, EFS.ERROR_DELETE);
-					Activator.getDefault().getBundle().getSymbolicName(),
-					getExceptionMessage(null, e), e));
 		}
 	}
 }

rse/plugins/org.eclipse.rse.services.local/src/org/eclipse/rse/internal/services/local/files/LocalFileService.java

@@ -38,7 +38,7 @@
  * Martin Oberhuber (Wind River) - [226262] Make IService IAdaptable
  * David McKnight   (IBM)        - [231211] Local xml file not opened when workspace encoding is different from local system encoding
  * Radoslav Gerganov (ProSyst)   - [230919] IFileService.delete() should not return a boolean
- * David McKnight   (IBM)        - [233373] NPE when deleting a file from a read-only folder on Local
+ * Martin Oberhuber (Wind River) - [233993] Improve EFS error reporting
  *******************************************************************************/
 
 package org.eclipse.rse.internal.services.local.files;
@@ -88,6 +88,7 @@ import org.eclipse.rse.services.clientserver.messages.SystemElementNotFoundExcep
 import org.eclipse.rse.services.clientserver.messages.SystemMessage;
 import org.eclipse.rse.services.clientserver.messages.SystemMessageException;
 import org.eclipse.rse.services.clientserver.messages.SystemOperationCancelledException;
+import org.eclipse.rse.services.clientserver.messages.SystemOperationFailedException;
 import org.eclipse.rse.services.files.AbstractFileService;
 import org.eclipse.rse.services.files.HostFilePermissions;
 import org.eclipse.rse.services.files.IFilePermissionsService;
@@ -476,7 +477,7 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 
 					String systemEncoding = SystemEncodingUtil.getInstance().getEnvironmentEncoding();
 					boolean sizeCheck = !isBinary && systemEncoding.equals(hostEncoding);
-					
+
 					if (sizeCheck && (destinationFile.length() != file.length())) {
 						throw new SystemOperationCancelledException();
 //						System.err.println("local.upload: size mismach on "+destinationFile.getAbsolutePath()); //$NON-NLS-1$
@@ -1048,17 +1049,15 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 		{
 			result = fileToDelete.delete();
 		}
-		if (!result){
+		if (!result) {
-		  if (fileToDelete.exists()) {
+			if (fileToDelete.exists()) {
-			// Deletion failed without specification why... likely a Security
+				// Deletion failed without specification why... likely a Security
-			// problem?
+				// problem, or an open file in the files to be deleted.
-			// TODO we'll want to wrap a message with the IOException at some point after
+				// TODO Externalize Message
-			// 3.0.1
+				throw new SystemOperationFailedException(Activator.PLUGIN_ID, "Failed to delete: " + fileToDelete.getAbsolutePath());
-			throw new RemoteFileSecurityException(new IOException());
+			} else {
-		  }
+				throw new SystemElementNotFoundException(fileToDelete.getAbsolutePath(), "delete");
-		  else {
+			}
-		  	throw new SystemElementNotFoundException(fileToDelete.getAbsolutePath(), "delete"); //$NON-NLS-1$	
-		  } 
 		}
 	}
 
@@ -1548,10 +1547,17 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 		}
 	}
 
-	public void setLastModified(String parent, String name, long timestamp, IProgressMonitor monitor)
+	public void setLastModified(String parent, String name, long timestamp, IProgressMonitor monitor) throws SystemMessageException
 	{
 		File file = new File(parent, name);
-		file.setLastModified(timestamp);
+		if (!file.setLastModified(timestamp)) {
+			if (!file.exists()) {
+				// TODO externalize message
+				throw new SystemElementNotFoundException(Activator.PLUGIN_ID, file.getAbsolutePath(), "setLastModified");
+			} else {
+				throw new SystemOperationFailedException(Activator.PLUGIN_ID, "setLastModified: " + file.getAbsolutePath());
+			}
+		}
 	}
 
 	public void setReadOnly(String parent, String name,
@@ -1559,10 +1565,11 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 	{
 		File file = new File(parent, name);
 		if (!file.exists()) {
-			String pluginId = Activator.PLUGIN_ID;
+			//TODO Externalize message, and/or centralize e.g. RemoteFileNotFoundException
-			String messageText = "File does not exist";
+			//See org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/Messages.java - fileNotFound
-			SimpleSystemMessage message = new SimpleSystemMessage(pluginId, IStatus.ERROR, messageText);
+			String messageText = "File not found";
-			throw new SystemMessageException(message);
+			//TODO throw new RemoteFileNotFoundException
+			throw new SystemElementNotFoundException(Activator.PLUGIN_ID, file.getAbsolutePath(), "setReadOnly");
 		}
 		if (readOnly != file.canWrite()) {
 			return;
@@ -1570,15 +1577,15 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 		if (readOnly)
 		{
 			if (!file.setReadOnly()) {
-				String pluginId = Activator.PLUGIN_ID;
+				//TODO Externalize message
-				String messageText = "Cannot set file read only";
+				throw new SystemOperationFailedException(Activator.PLUGIN_ID, "Failed to setReadOnly: " + file.getAbsolutePath());
-				SimpleSystemMessage message = new SimpleSystemMessage(pluginId, IStatus.ERROR, messageText);
-				throw new SystemMessageException(message);
 			}
 			return;
 		}
 		else
 		{
+			Exception remoteException = null;
+			String remoteError = ""; //$NON-NLS-1$
 			if (!_isWindows)
 			{
 				// make this read-write
@@ -1591,15 +1598,19 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 				{
 					Process p = Runtime.getRuntime().exec(cmd);
 					exitValue = p.waitFor();
+					if (p.getErrorStream().available() > 0) {
+						remoteError = ": " + new BufferedReader(new InputStreamReader(p.getErrorStream())).readLine(); //$NON-NLS-1$
+					} else if (p.getInputStream().available() > 0) {
+						remoteError = ": " + new BufferedReader(new InputStreamReader(p.getInputStream())).readLine(); //$NON-NLS-1$
+					}
 				}
 				catch (Exception e)
 				{
+					remoteException = e;
 				}
 				if (exitValue != 0) {
-					String pluginId = Activator.PLUGIN_ID;
+					//TODO Externalize message
-					String messageText = "Cannot set file read-write";
+					throw new SystemOperationFailedException(Activator.PLUGIN_ID, "Failed to setWritable: " + remoteError, remoteException);
-					SimpleSystemMessage message = new SimpleSystemMessage(pluginId, IStatus.ERROR, messageText);
-					throw new SystemMessageException(message);
 				}
 			}
 			// windows version
@@ -1614,17 +1625,31 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 				{
 					Process p = Runtime.getRuntime().exec(cmd);
 					exitValue = p.waitFor();
+					if (p.getErrorStream().available() > 0) {
+						remoteError = ": " + new BufferedReader(new InputStreamReader(p.getErrorStream())).readLine(); //$NON-NLS-1$
+					} else if (p.getInputStream().available() > 0) {
+						remoteError = ": " + new BufferedReader(new InputStreamReader(p.getInputStream())).readLine(); //$NON-NLS-1$
+					}
 				}
 				catch (Exception e)
 				{
+					remoteException = e;
 				}
 				if (exitValue != 0) {
-					String pluginId = Activator.PLUGIN_ID;
+					//TODO Externalize String
-					String messageText = "Cannot set file read-write";
+					throw new SystemOperationFailedException(Activator.PLUGIN_ID, "Failed to setWritable: " + remoteError, remoteException);
-					SimpleSystemMessage message = new SimpleSystemMessage(pluginId, IStatus.ERROR, messageText);
-					throw new SystemMessageException(message);
 				}
 			}
+			//Verify that it actually worked
+			if (!file.canWrite()) {
+				if (remoteError.length() == 0) {
+					// TODO Externalize String
+					remoteError = "Failed to setWritable: " + file.getAbsolutePath();
+				} else {
+					remoteError = remoteError.substring(2);
+				}
+				throw new SystemOperationFailedException(Activator.PLUGIN_ID, remoteError);
+			}
 		}
 	}
 
@@ -1641,6 +1666,13 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 		try {
 			stream = new FileInputStream(file);
 		}
+		catch (FileNotFoundException e) {
+			if (!file.exists()) {
+				throw new SystemElementNotFoundException(Activator.PLUGIN_ID, file.getAbsolutePath(), "getInputStream");
+			} else {
+				throw new RemoteFileIOException(e);
+			}
+		}
 		catch (Exception e) {
 			throw new RemoteFileIOException(e);
 		}
@@ -1673,6 +1705,13 @@ public class LocalFileService extends AbstractFileService implements ILocalServi
 				stream = new FileOutputStream(file, true);
 			}
 		}
+		catch (FileNotFoundException e) {
+			if (!file.exists()) {
+				throw new SystemElementNotFoundException(Activator.PLUGIN_ID, file.getAbsolutePath(), "getOutputStream");
+			} else {
+				throw new RemoteFileIOException(e);
+			}
+		}
 		catch (Exception e) {
 			throw new RemoteFileIOException(e);
 		}

rse/plugins/org.eclipse.rse.services/clientserver/org/eclipse/rse/services/clientserver/messages/SystemElementNotFoundException.java

@@ -7,6 +7,7 @@
  *
  * Contributors:
  * Martin Oberhuber (Wind River) - initial API and implementation
+ * Martin Oberhuber (Wind River) - [233993] Improve EFS error reporting
  *******************************************************************************/
 
 package org.eclipse.rse.services.clientserver.messages;
@@ -21,7 +22,7 @@ import org.eclipse.rse.services.clientserver.IClientServerConstants;
  * but that element did not exist. Like trying to delete a file that does not
  * exist. The framework may treat such an exception differently than other kinds
  * of exceptions.
- * 
+ *
  * @since 3.0
  */
 public class SystemElementNotFoundException extends SystemMessageException {
@@ -32,19 +33,40 @@ public class SystemElementNotFoundException extends SystemMessageException {
 	 */
 	private static final long serialVersionUID = 1L;
 
+	/**
+	 * Constructor for subclasses to use custom messages.
+	 *
+	 * @param msg A SystemMessage.
+	 */
+	public SystemElementNotFoundException(SystemMessage msg) {
+		super(msg);
+	}
+
 	/**
 	 * Constructor.
 	 *
 	 * @param element an Absolute Path for the element that could not be found.
+	 * @param operation Operation about to be performed that failed
 	 */
 	public SystemElementNotFoundException(String element, String operation) {
-		super(getMyMessage(element, operation));
+		super(getMyMessage(IClientServerConstants.PLUGIN_ID, element, operation));
 	}
 
-	private static SystemMessage getMyMessage(String element, String operation) {
+	/**
-		//TODO generate an internal backtrace and attach to the message
+	 * Constructor.
+	 *
+	 * @param pluginId ID of plugin in which the failure occurred
+	 * @param element an Absolute Path for the element that could not be found.
+	 * @param operation Operation about to be performed that failed
+	 */
+	public SystemElementNotFoundException(String pluginId, String element, String operation) {
+		super(getMyMessage(pluginId, element, operation));
+	}
+
+	private static SystemMessage getMyMessage(String pluginId, String element, String operation) {
+		//TODO generate an internal backtrace and attach to the message?
 		String msgText = NLS.bind(CommonMessages.MSG_ELEMENT_NOT_FOUND, operation, element);
-		SystemMessage msg = new SimpleSystemMessage(IClientServerConstants.PLUGIN_ID,
+		SystemMessage msg = new SimpleSystemMessage(pluginId,
 				ICommonMessageIds.MSG_ELEMENT_NOT_FOUND, IStatus.ERROR, msgText);
 		return msg;
 	}