Backup no longer shows errors on success

Tests added that prove errors are being displayed on successful backup
Resolved

community/kernel/src/test/java/org/neo4j/test/ProcessStreamHandler.java

@@ -54,16 +54,15 @@ public ProcessStreamHandler( Process process, boolean quiet )
         this( process, quiet, "", quiet ? IGNORE_FAILURES : PRINT_FAILURES );
     }
 
-    public ProcessStreamHandler( Process process, boolean quiet, String prefix,
-            StreamExceptionHandler failureHandler )
+    public ProcessStreamHandler( Process process, boolean quiet, String prefix, StreamExceptionHandler failureHandler )
     {
         this( process, quiet, prefix, failureHandler, System.out, System.err );
     }
 
-    public ProcessStreamHandler( Process process, boolean quiet, String prefix,
-            StreamExceptionHandler failureHandler, PrintStream out, PrintStream err )
+    public ProcessStreamHandler( Process process, boolean quiet, String prefix, StreamExceptionHandler failureHandler, PrintStream out, PrintStream err )
     {
         this.process = process;
+
         this.out = new Thread( new StreamConsumer( process.getInputStream(), out, quiet, prefix, failureHandler ) );
         this.err = new Thread( new StreamConsumer( process.getErrorStream(), err, quiet, prefix, failureHandler ) );
     }

enterprise/backup/src/main/java/org/neo4j/backup/impl/CausalClusteringBackupStrategy.java

@@ -63,7 +63,7 @@ public Fallible<BackupStageOutcome> performFullBackup( Path desiredBackupLocatio
 
         try
         {
-            backupDelegator.copy( fromAddress,storeId, desiredBackupLocation );
+            backupDelegator.copy( fromAddress, storeId, desiredBackupLocation );
             return new Fallible<>( BackupStageOutcome.SUCCESS, null );
         }
         catch ( StoreCopyFailedException e )

enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupCommandBuilder.java

@@ -64,6 +64,13 @@ public class OnlineBackupCommandBuilder
     private Boolean consistencyCheckLabel;
     private Boolean consistencyCheckOwners;
     private OutputStream output;
+    private Optional<String[]> rawArgs = Optional.empty();
+
+    public OnlineBackupCommandBuilder withRawArgs( String... args )
+    {
+        rawArgs = Optional.of( args );
+        return this;
+    }
 
     public OnlineBackupCommandBuilder withHost( String host )
     {
@@ -147,13 +154,20 @@ public boolean backup( File neo4jHome, String backupName ) throws CommandFailed,
     {
         File targetLocation = new File( neo4jHome, backupName );
         String[] args;
-        try
+        if ( rawArgs.isPresent() )
         {
-            args = resolveArgs( targetLocation );
+            args = rawArgs.get();
         }
-        catch ( IOException e )
+        else
         {
-            throw new CommandFailed( "Failed to resolve arguments", e );
+            try
+            {
+                args = resolveArgs( targetLocation );
+            }
+            catch ( IOException e )
+            {
+                throw new CommandFailed( "Failed to resolve arguments", e );
+            }
         }
         new OnlineBackupCommandProvider()
                 .create( neo4jHome.toPath(),

enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupCommandProvider.java

@@ -30,6 +30,7 @@
 import org.neo4j.commandline.arguments.Arguments;
 import org.neo4j.kernel.monitoring.Monitors;
 import org.neo4j.logging.FormattedLogProvider;
+import org.neo4j.logging.Level;
 import org.neo4j.logging.LogProvider;
 
 import static java.lang.String.format;
@@ -81,7 +82,8 @@ public AdminCommandSection commandSection()
     @Nonnull
     public AdminCommand create( Path homeDir, Path configDir, OutsideWorld outsideWorld )
     {
-        LogProvider logProvider = FormattedLogProvider.toOutputStream( outsideWorld.outStream() );
+        boolean debug = System.getenv().get( "NEO4J_DEBUG") != null;
+        LogProvider logProvider = FormattedLogProvider.withDefaultLogLevel( debug ? Level.DEBUG : Level.NONE ).toOutputStream( outsideWorld.outStream() );
         Monitors monitors = new Monitors();
 
         OnlineBackupContextBuilder contextBuilder = new OnlineBackupContextBuilder( homeDir, configDir );

enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandCcIT.java

@@ -30,7 +30,10 @@
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.IOException;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -57,6 +60,7 @@
 import org.neo4j.util.TestHelpers;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assume.assumeFalse;
 
 @RunWith( Parameterized.class )
@@ -180,7 +184,7 @@ public void secondaryTransactionProtocolIsSwitchedOffCorrespondingBackupSetting(
         String value = "localhost:%d";
         clusterRule = clusterRule.withSharedCoreParam( OnlineBackupSettings.online_backup_enabled, "false" )
                 .withInstanceCoreParam( OnlineBackupSettings.online_backup_server, i -> String.format( value, backupPorts[i] ) );
-        Cluster cluster = startCluster( recordFormat );
+        startCluster( recordFormat );
         String customAddress = "localhost:" + backupPorts[0];
 
         // then a full backup is impossible from the backup port
@@ -204,6 +208,86 @@ public void backupNotPossibleIfLegacyProtocolSpecified() throws Exception
                         "--proto=legacy") );
     }
 
+    @Test
+    public void backupDoesntDisplayExceptionWhenSuccessful() throws Exception
+    {
+        // given
+        Cluster cluster = startCluster( recordFormat );
+        String customAddress = CausalClusteringTestHelpers.transactionAddress( clusterLeader( cluster ).database() );
+
+        // and
+        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+        PrintStream outputStream = wrapWithNormalOutput( System.out, new PrintStream( byteArrayOutputStream ) );
+
+        // and
+        ByteArrayOutputStream byteArrayErrorStream = new ByteArrayOutputStream();
+        PrintStream errorStream = wrapWithNormalOutput( System.err, new PrintStream( byteArrayErrorStream ) );
+
+        // when
+        assertEquals(
+                0,
+                runBackupToolFromOtherJvmToGetExitCode( outputStream, errorStream,
+                        "--from", customAddress,
+                        "--cc-report-dir=" + backupDir,
+                        "--backup-dir=" + backupDir,
+                        "--name=defaultport" ) );
+
+        // then
+        assertFalse( byteArrayErrorStream.toString().toLowerCase().contains( "exception" ) );
+        assertFalse( byteArrayOutputStream.toString().toLowerCase().contains( "exception" ) );
+    }
+
+    static PrintStream wrapWithNormalOutput( PrintStream normalOutput, PrintStream nullAbleOutput )
+    {
+        if ( nullAbleOutput == null )
+        {
+            return normalOutput;
+        }
+        return duplexPrintStream( normalOutput, nullAbleOutput );
+    }
+
+    private static PrintStream duplexPrintStream( PrintStream first, PrintStream second )
+    {
+        return new PrintStream( first )
+        {
+
+            @Override
+            public void write( int i )
+            {
+                super.write( i );
+                second.write( i );
+            }
+
+            @Override
+            public void write( byte[] bytes, int i, int i1 )
+            {
+                super.write( bytes, i, i1 );
+                second.write( bytes, i, i1 );
+            }
+
+            @Override
+            public void write( byte[] bytes ) throws IOException
+            {
+                super.write( bytes );
+                second.write( bytes );
+            }
+
+            @Override
+            public void flush()
+            {
+                super.flush();
+                second.flush();
+            }
+
+            @Override
+            public void close()
+            {
+                super.close();
+                second.close();
+            }
+        };
+    }
+
     private void repeatedlyPopulateDatabase( Cluster cluster, AtomicBoolean continueFlagReference )
     {
         while ( continueFlagReference.get() )
@@ -217,14 +301,12 @@ public static CoreGraphDatabase clusterDatabase( Cluster cluster )
         return clusterLeader( cluster ).database();
     }
 
-    private Cluster startCluster( String recordFormat )
-            throws Exception
+    private Cluster startCluster( String recordFormat ) throws Exception
     {
-        ClusterRule clusterRule = this.clusterRule
-                .withSharedCoreParam( GraphDatabaseSettings.record_format, recordFormat )
+        ClusterRule clusterRule = this.clusterRule.withSharedCoreParam( GraphDatabaseSettings.record_format, recordFormat )
                 .withSharedReadReplicaParam( GraphDatabaseSettings.record_format, recordFormat );
         Cluster cluster = clusterRule.startCluster();
-        createSomeData(  cluster );
+        createSomeData( cluster );
         return cluster;
     }
 
@@ -253,6 +335,11 @@ public static DbRepresentation getBackupDbRepresentation( String name, File back
         return DbRepresentation.of( new File( backupDir, name ), config );
     }
 
+    private int runBackupToolFromOtherJvmToGetExitCode( PrintStream outputStream, PrintStream errorStream, String... args ) throws Exception
+    {
+        return TestHelpers.runBackupToolFromOtherJvmToGetExitCode( testDirectory.absolutePath(), outputStream, errorStream, false, args );
+    }
+
     private int runBackupToolFromOtherJvmToGetExitCode( String... args ) throws Exception
     {
         return TestHelpers.runBackupToolFromOtherJvmToGetExitCode( testDirectory.absolutePath(), args );

enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java

@@ -23,7 +23,6 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
@@ -32,7 +31,9 @@
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -61,9 +62,11 @@
 
 import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
+import static org.neo4j.backup.impl.OnlineBackupCommandCcIT.wrapWithNormalOutput;
 import static org.neo4j.util.TestHelpers.runBackupToolFromOtherJvmToGetExitCode;
 
 @RunWith( Parameterized.class )
@@ -204,6 +207,36 @@ public void backupFailsWithCatchupProtoOverride() throws Exception
                         "--name=" + backupName ) );
     }
 
+    @Test
+    public void backupDoesNotDisplayExceptionWhenSuccessful() throws Exception
+    {
+        // given
+        String backupName = "customPort" + recordFormat;
+        int backupPort = PortAuthority.allocatePort();
+        startDb( backupPort );
+
+        // and
+        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+        PrintStream outputStream = wrapWithNormalOutput( System.out, new PrintStream( byteArrayOutputStream ) );
+
+        // and
+        ByteArrayOutputStream byteArrayErrorStream = new ByteArrayOutputStream();
+        PrintStream errorStream = wrapWithNormalOutput( System.err, new PrintStream( byteArrayErrorStream ) );
+
+        // when
+        assertEquals(
+                0,
+                runBackupTool( outputStream, errorStream,
+                        "--from", "127.0.0.1:" + backupPort,
+                        "--cc-report-dir=" + backupDir,
+                        "--backup-dir=" + backupDir,
+                        "--name=" + backupName ) );
+
+        // then
+        assertFalse( byteArrayErrorStream.toString().toLowerCase().contains( "exception" ) );
+        assertFalse( byteArrayOutputStream.toString().toLowerCase().contains( "exception" ) );
+    }
+
     private void repeatedlyPopulateDatabase( GraphDatabaseService db, AtomicBoolean continueFlagReference )
     {
         while ( continueFlagReference.get() )
@@ -237,6 +270,11 @@ private void startDb( Integer backupPort )
         createSomeData( db );
     }
 
+    private static int runBackupTool( PrintStream outputStream, PrintStream errorStream, String... args ) throws Exception
+    {
+        return runBackupToolFromOtherJvmToGetExitCode( testDirectory.absolutePath(), outputStream, errorStream, false, args );
+    }
+
     private static int runBackupTool( String... args )
             throws Exception
     {

enterprise/kernel/src/test/java/org/neo4j/util/TestHelpers.java

@@ -20,6 +20,7 @@
 package org.neo4j.util;
 
 import java.io.File;
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -32,6 +33,7 @@
 import org.neo4j.kernel.impl.enterprise.configuration.OnlineBackupSettings;
 import org.neo4j.kernel.internal.GraphDatabaseAPI;
 import org.neo4j.test.ProcessStreamHandler;
+import org.neo4j.test.StreamConsumer;
 
 import static java.lang.String.format;
 import static org.neo4j.kernel.configuration.Settings.listenAddress;
@@ -61,6 +63,12 @@ public static <E extends Exception> E executionIsExpectedToFail( Runnable runnab
     }
 
     public static int runBackupToolFromOtherJvmToGetExitCode( File neo4jHome, String... args ) throws Exception
+    {
+        return runBackupToolFromOtherJvmToGetExitCode( neo4jHome, System.out, System.err, true, args );
+    }
+
+    public static int runBackupToolFromOtherJvmToGetExitCode( File neo4jHome, PrintStream outPrintStream, PrintStream errPrintStream,
+            boolean debug, String... args ) throws Exception
     {
         List<String> allArgs =
                 new ArrayList<>( Arrays.asList( ProcessUtil.getJavaExecutable().toString(), "-cp", ProcessUtil.getClassPath(), AdminTool.class.getName() ) );
@@ -69,15 +77,20 @@ public static int runBackupToolFromOtherJvmToGetExitCode( File neo4jHome, String
 
         ProcessBuilder processBuilder = new ProcessBuilder().command( allArgs.toArray( new String[allArgs.size()]));
         processBuilder.environment().put( "NEO4J_HOME", neo4jHome.getAbsolutePath() );
-        processBuilder.environment().put( "NEO4J_DEBUG", "anything_works" );
+        if ( debug )
+        {
+            processBuilder.environment().put( "NEO4J_DEBUG", "anything_works" );
+        }
         Process process = processBuilder.start();
-        return new ProcessStreamHandler( process, false ).waitForResult();
+        ProcessStreamHandler processStreamHandler =
+                new ProcessStreamHandler( process, false, "", StreamConsumer.IGNORE_FAILURES, outPrintStream, errPrintStream );
+        return processStreamHandler.waitForResult();
     }
 
     public static String backupAddressCc( GraphDatabaseAPI graphDatabase )
     {
         ListenSocketAddress hostnamePort = graphDatabase.getDependencyResolver().resolveDependency( Config.class ).get(
-                listenAddress( "causal_clustering.transaction_listen_address", 6000 ) );
+                listenAddress( "dbms.backup.address", 6000 ) );
 
         return hostnamePort.toString();
     }