Propagate failure of ssh command to caller of SshSupport

When SshSupport.runSshCommand fails since the executed external ssh
command failed throw a CommandFailedException.

If discovery of LFS server fails due to failure of the
git-lfs-authenticate command chain the CommandFailureException to the
LfsConfigInvalidException in order to allow root cause analysis in the
application using that.

Change-Id: I2f9ea2be11274549f6d845937164c248b3d840b2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
index 91f0bdb0945790ff36d7aa8048a2627ad8ad9efa..0e00f146aec5e3f874990444cffddc988b2a19f5 100644 (file)
--- a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
+++ b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties
@@ -1,4 +1,3 @@
-cannotDiscoverLfs=Cannot discover LFS URI
 corruptLongObject=The content hash ''{0}'' of the long object ''{1}'' doesn''t match its id, the corrupt object will be deleted.
 incorrectLONG_OBJECT_ID_LENGTH=Incorrect LONG_OBJECT_ID_LENGTH.
 inconsistentMediafileLength=Mediafile {0} has unexpected length; expected {1} but found {2}.

diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java
index 5320af0b7856ae7b3791e38bfeb0d05edcd7c1b1..f90a003b4b8cf331ccf3c303d585f0dc084e4f37 100644 (file)
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java
@@ -63,4 +63,17 @@ public class LfsConfigInvalidException extends IOException {
                super(msg);
        }
 
+       /**
+        * Constructor for LfsConfigInvalidException.
+        *
+        * @param msg
+        *            the error description
+        * @param e
+        *            cause of this exception
+        * @since 5.0
+        */
+       public LfsConfigInvalidException(String msg, Exception e) {
+               super(msg, e);
+       }
+
 }

diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
index bac980bb8d8409c542aba0a557768fcf398a711b..955eca0c3f2650db8f532244edbe1af2eb90f570 100644 (file)
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
@@ -49,6 +49,7 @@ import static org.eclipse.jgit.util.HttpSupport.HDR_CONTENT_TYPE;
 
 import java.io.IOException;
 import java.net.ProxySelector;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.text.SimpleDateFormat;
 import java.util.LinkedList;
@@ -56,6 +57,7 @@ import java.util.Map;
 import java.util.TreeMap;
 
 import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.errors.CommandFailedException;
 import org.eclipse.jgit.lfs.LfsPointer;
 import org.eclipse.jgit.lfs.Protocol;
 import org.eclipse.jgit.lfs.errors.LfsConfigInvalidException;
@@ -68,17 +70,12 @@ import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.transport.http.HttpConnection;
 import org.eclipse.jgit.util.HttpSupport;
 import org.eclipse.jgit.util.SshSupport;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Provides means to get a valid LFS connection for a given repository.
  */
 public class LfsConnectionFactory {
 
-       private static final Logger log = LoggerFactory
-                       .getLogger(LfsConnectionFactory.class);
-
        private static final int SSH_AUTH_TIMEOUT_SECONDS = 30;
        private static final String SCHEME_HTTPS = "https"; //$NON-NLS-1$
        private static final String SCHEME_SSH = "ssh"; //$NON-NLS-1$
@@ -133,6 +130,7 @@ public class LfsConnectionFactory {
                String lfsUrl = config.getString(ConfigConstants.CONFIG_SECTION_LFS,
                                null,
                                ConfigConstants.CONFIG_KEY_URL);
+               Exception ex = null;
                if (lfsUrl == null) {
                        String remoteUrl = null;
                        for (String remote : db.getRemoteNames()) {
@@ -151,39 +149,44 @@ public class LfsConnectionFactory {
                                break;
                        }
                        if (lfsUrl == null && remoteUrl != null) {
-                               lfsUrl = discoverLfsUrl(db, purpose, additionalHeaders,
-                                               remoteUrl);
+                               try {
+                                       lfsUrl = discoverLfsUrl(db, purpose, additionalHeaders,
+                                                       remoteUrl);
+                               } catch (URISyntaxException | IOException
+                                               | CommandFailedException e) {
+                                       ex = e;
+                               }
                        } else {
                                lfsUrl = lfsUrl + Protocol.INFO_LFS_ENDPOINT;
                        }
                }
                if (lfsUrl == null) {
+                       if (ex != null) {
+                               throw new LfsConfigInvalidException(
+                                               LfsText.get().lfsNoDownloadUrl, ex);
+                       }
                        throw new LfsConfigInvalidException(LfsText.get().lfsNoDownloadUrl);
                }
                return lfsUrl;
        }
 
        private static String discoverLfsUrl(Repository db, String purpose,
-                       Map<String, String> additionalHeaders, String remoteUrl) {
-               try {
-                       URIish u = new URIish(remoteUrl);
-                       if (u.getScheme() == null || SCHEME_SSH.equals(u.getScheme())) {
-                               Protocol.ExpiringAction action = getSshAuthentication(
-                                               db, purpose, remoteUrl, u);
-                               additionalHeaders.putAll(action.header);
-                               return action.href;
-                       } else {
-                               return remoteUrl + Protocol.INFO_LFS_ENDPOINT;
-                       }
-               } catch (Exception e) {
-                       log.error(LfsText.get().cannotDiscoverLfs, e);
-                       return null; // could not discover
+                       Map<String, String> additionalHeaders, String remoteUrl)
+                       throws URISyntaxException, IOException, CommandFailedException {
+               URIish u = new URIish(remoteUrl);
+               if (u.getScheme() == null || SCHEME_SSH.equals(u.getScheme())) {
+                       Protocol.ExpiringAction action = getSshAuthentication(db, purpose,
+                                       remoteUrl, u);
+                       additionalHeaders.putAll(action.header);
+                       return action.href;
+               } else {
+                       return remoteUrl + Protocol.INFO_LFS_ENDPOINT;
                }
        }
 
        private static Protocol.ExpiringAction getSshAuthentication(
                        Repository db, String purpose, String remoteUrl, URIish u)
-                       throws IOException {
+                       throws IOException, CommandFailedException {
                AuthCache cached = sshAuthCache.get(remoteUrl);
                Protocol.ExpiringAction action = null;
                if (cached != null && cached.validUntil > System.currentTimeMillis()) {

diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
index d51203a740d6c6de509ca03bff48ddd2a6c73cd9..d7d0fe186aaade1693a73da8c339dac215ac1caf 100644 (file)
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java
@@ -60,7 +60,6 @@ public class LfsText extends TranslationBundle {
        }
 
        // @formatter:off
-       /***/ public String cannotDiscoverLfs;
        /***/ public String corruptLongObject;
        /***/ public String inconsistentMediafileLength;
        /***/ public String inconsistentContentLength;

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index b608ca8533f85e1b9f4f3a73beb75a857b7ebc1a..809bdf2865a16f6ccdb3ca88ced4cbbff3e4fbd9 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -629,6 +629,7 @@ sourceIsNotAWildcard=Source is not a wildcard.
 sourceRefDoesntResolveToAnyObject=Source ref {0} doesn''t resolve to any object.
 sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0}
 squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD
+sshCommandFailed=Execution of ssh command ''{0}'' failed with error ''{1}''
 sshUserNameError=Jsch error: failed to set SSH user name correctly to ''{0}''; using ''{1}'' picked up from SSH config file.
 sslFailureExceptionMessage=Secure connection to {0} could not be stablished because of SSL problems
 sslFailureInfo=A secure connection to {0}\ncould not be established because the server''s certificate could not be validated.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 2ac75e1c2fb3262e680fe0c53205296cd48c8aec..f3029bf3a13438e5209cfcc6544c93f8df8a0e1d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -690,6 +690,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String sourceRefDoesntResolveToAnyObject;
        /***/ public String sourceRefNotSpecifiedForRefspec;
        /***/ public String squashCommitNotUpdatingHEAD;
+       /***/ public String sshCommandFailed;
        /***/ public String sshUserNameError;
        /***/ public String sslFailureExceptionMessage;
        /***/ public String sslFailureInfo;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
index 40a576ed22fd66193582cb7930a27d9d6ac83ca3..913aa728675f6bed2bf21189948e4a109b9a16da 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
@@ -43,8 +43,11 @@
 package org.eclipse.jgit.util;
 
 import java.io.IOException;
+import java.text.MessageFormat;
 
 import org.eclipse.jgit.annotations.Nullable;
+import org.eclipse.jgit.errors.CommandFailedException;
+import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.RemoteSession;
 import org.eclipse.jgit.transport.SshSessionFactory;
@@ -75,18 +78,23 @@ public class SshSupport {
         * @param timeout
         *            a timeout in seconds. The timeout may be exceeded in corner
         *            cases.
-        * @return The entire output read from stdout. Stderr is discarded.
+        * @return The entire output read from stdout.
         * @throws IOException
+        * @throws CommandFailedException
+        *             if the ssh command execution failed, error message contains
+        *             the content of stderr.
         */
        public static String runSshCommand(URIish sshUri,
                        @Nullable CredentialsProvider provider, FS fs, String command,
-                       int timeout) throws IOException {
+                       int timeout) throws IOException, CommandFailedException {
                RemoteSession session = null;
                Process process = null;
                StreamCopyThread errorThread = null;
                StreamCopyThread outThread = null;
-               try (MessageWriter stderr = new MessageWriter();
-                               MessageWriter stdout = new MessageWriter()) {
+               CommandFailedException failure = null;
+               @SuppressWarnings("resource")
+               MessageWriter stderr = new MessageWriter();
+               try (MessageWriter stdout = new MessageWriter()) {
                        session = SshSessionFactory.getInstance().getSession(sshUri,
                                        provider, fs, 1000 * timeout);
                        process = session.exec(command, 0);
@@ -127,11 +135,21 @@ public class SshSupport {
                                }
                        }
                        if (process != null) {
+                               if (process.exitValue() != 0) {
+                                       failure = new CommandFailedException(process.exitValue(),
+                                                       MessageFormat.format(
+                                                       JGitText.get().sshCommandFailed, command,
+                                                       stderr.toString()));
+                               }
                                process.destroy();
                        }
+                       stderr.close();
                        if (session != null) {
                                SshSessionFactory.getInstance().releaseSession(session);
                        }
+                       if (failure != null) {
+                               throw failure;
+                       }
                }
        }