From 0b60ccf5ac7d9a3d3918567befb5a04f4daaf3f7 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 23 Jul 2011 15:33:50 +1000 Subject: [PATCH] make it explicit we are not going to retry on a general sftp failure --- .../org/jclouds/ssh/jsch/JschSshClient.java | 28 ++++++++++++------- .../jclouds/ssh/jsch/JschSshClientTest.java | 20 +++++++++++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java b/drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java index f5b4cda3dc..cb02eea5d7 100644 --- a/drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java +++ b/drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java @@ -24,7 +24,6 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Predicates.instanceOf; import static com.google.common.base.Predicates.or; import static com.google.common.base.Throwables.getCausalChain; -import static com.google.common.base.Throwables.getRootCause; import static com.google.common.collect.Iterables.any; import java.io.IOException; @@ -53,7 +52,6 @@ import org.jclouds.util.Strings2; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; import com.google.common.io.Closeables; import com.google.inject.Inject; import com.jcraft.jsch.ChannelExec; @@ -244,7 +242,7 @@ public class JschSshClient implements SshClient { public void throwChannelNotOpenExceptionOrPropagate(String channel, JSchException e) throws JSchException { if (e.getMessage().indexOf("channel is not opened") != -1) - throw new ChannelNotOpenException(String.format("(%s) channel %s is not open", toString() , channel), e); + throw new ChannelNotOpenException(String.format("(%s) channel %s is not open", toString(), channel), e); } class GetConnection implements Connection { @@ -296,8 +294,9 @@ public class JschSshClient implements SshClient { @Override public Void create() throws Exception { sftp = acquire(sftpConnection); + InputStream is = checkNotNull(contents.getInput(), "inputstream for path %s", path); try { - sftp.put(checkNotNull(contents.getInput(), "inputstream for path %s", path), path); + sftp.put(is, path); } finally { Closeables.closeQuietly(contents); clear(); @@ -318,18 +317,27 @@ public class JschSshClient implements SshClient { @VisibleForTesting boolean shouldRetry(Exception from) { - final String rootMessage = getRootCause(from).getMessage(); - if (rootMessage == null) - return false; return any(getCausalChain(from), retryPredicate) - || Iterables.any(Splitter.on(",").split(retryableMessages), new Predicate() { + || any(Splitter.on(",").split(retryableMessages), causalChainHasMessageContaining(from)); + } + + @VisibleForTesting + Predicate causalChainHasMessageContaining(final Exception from) { + return new Predicate() { + + @Override + public boolean apply(final String input) { + return any(getCausalChain(from), new Predicate() { @Override - public boolean apply(String input) { - return rootMessage.indexOf(input) != -1; + public boolean apply(Throwable arg0) { + return arg0.getMessage() != null && arg0.getMessage().indexOf(input) != -1; } }); + } + + }; } private void backoffForAttempt(int retryAttempt, String message) { diff --git a/drivers/jsch/src/test/java/org/jclouds/ssh/jsch/JschSshClientTest.java b/drivers/jsch/src/test/java/org/jclouds/ssh/jsch/JschSshClientTest.java index dcd4a484fa..324bc96b2a 100644 --- a/drivers/jsch/src/test/java/org/jclouds/ssh/jsch/JschSshClientTest.java +++ b/drivers/jsch/src/test/java/org/jclouds/ssh/jsch/JschSshClientTest.java @@ -32,7 +32,9 @@ import org.testng.annotations.Test; import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.Module; +import com.jcraft.jsch.ChannelSftp; import com.jcraft.jsch.JSchException; +import com.jcraft.jsch.SftpException; /** * @@ -64,6 +66,8 @@ public class JschSshClientTest { assert ssh.shouldRetry(new JSchException("io error", new IOException("socket closed"))); assert ssh.shouldRetry(new JSchException("connect error", new ConnectException("problem"))); + assert ssh.shouldRetry(new IOException("channel %s is not open", new NullPointerException())); + assert ssh.shouldRetry(new IOException("channel %s is not open", new NullPointerException(null))); } @@ -72,4 +76,20 @@ public class JschSshClientTest { assert ssh.shouldRetry(new JSchException("Session.connect: invalid data")); assert ssh.shouldRetry(new JSchException("Session.connect: java.net.SocketException: Connection reset")); } + + public void testDoNotRetryOnGeneralSftpError() { + // http://sourceforge.net/mailarchive/forum.php?thread_name=CAARMrHVhASeku48xoAgWEb-nEpUuYkMA03PoA5TvvFdk%3DjGKMA%40mail.gmail.com&forum_name=jsch-users + assert !ssh.shouldRetry(new SftpException(ChannelSftp.SSH_FX_FAILURE, new NullPointerException().toString())); + } + + public void testCausalChainHasMessageContaining() { + assert ssh.causalChainHasMessageContaining( + new JSchException("Session.connect: java.io.IOException: End of IO Stream Read")).apply( + " End of IO Stream Read"); + assert ssh.causalChainHasMessageContaining(new JSchException("Session.connect: invalid data")).apply( + " invalid data"); + assert ssh.causalChainHasMessageContaining( + new JSchException("Session.connect: java.net.SocketException: Connection reset")).apply("java.net.Socket"); + assert !ssh.causalChainHasMessageContaining(new NullPointerException()).apply(" End of IO Stream Read"); + } } \ No newline at end of file