From 0c4907d54a1b777ffe908494f8b01041a9ef6452 Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Tue, 14 Mar 2017 12:29:44 +0100 Subject: [PATCH] Exit with different error codes when consistency-check fails --- .../neo4j/commandline/admin/AdminTool.java | 13 ++- .../commandline/admin/CommandFailed.java | 20 ++++- .../org/neo4j/backup/OnlineBackupCommand.java | 10 ++- .../neo4j/backup/OnlineBackupCommandTest.java | 81 ++++++++++++++++--- .../dbms/UnbindFromClusterCommand.java | 8 +- 5 files changed, 107 insertions(+), 25 deletions(-) diff --git a/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminTool.java b/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminTool.java index aa30eada246f8..f7c73c3016566 100644 --- a/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminTool.java +++ b/community/command-line/src/main/java/org/neo4j/commandline/admin/AdminTool.java @@ -149,7 +149,7 @@ private void unexpected( RuntimeException e ) private void commandFailed( CommandFailed e ) { - failure( "command failed", e ); + failure( "command failed", e, e.code() ); } private void failure() @@ -158,18 +158,23 @@ private void failure() } private void failure( String message, Exception e ) + { + failure( message, e, 1 ); + } + + private void failure( String message, Exception e, int code ) { if ( debug ) { outsideWorld.printStacktrace( e ); } - failure( format( "%s: %s", message, e.getMessage() ) ); + failure( format( "%s: %s", message, e.getMessage() ), code ); } - private void failure( String message ) + private void failure( String message, int code ) { outsideWorld.stdErrLine( message ); - outsideWorld.exit( 1 ); + outsideWorld.exit( code ); } private void success() diff --git a/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java b/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java index e3ba040a4e8f4..8a4f8ddb02abc 100644 --- a/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java +++ b/community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java @@ -21,18 +21,32 @@ public class CommandFailed extends Exception { - public CommandFailed( String message, Exception cause ) + private final int code; + + public CommandFailed( String message, Throwable cause, int code ) { super( message, cause ); + this.code = code; } - public CommandFailed( Throwable cause ) + public CommandFailed( String message, Throwable cause ) { - super( cause ); + this( message, cause, 1 ); } public CommandFailed( String message ) + { + this( message, 1 ); + } + + public CommandFailed( String message, int code ) { super( message ); + this.code = code; + } + + public int code() + { + return code; } } diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java b/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java index c2594623bb3c9..bad751fc42329 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java @@ -249,12 +249,16 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed if ( !ccResult.isSuccessful() ) { throw new CommandFailed( String.format( "Inconsistencies found. See '%s' for details.", - ccResult.reportFile() ) ); + ccResult.reportFile() ), 3 ); } } - catch ( Exception e ) + catch ( Throwable e ) { - throw new CommandFailed( "Failed to do consistency check on backup: " + e.getMessage(), e ); + if ( e instanceof CommandFailed ) + { + throw (CommandFailed) e; + } + throw new CommandFailed( "Failed to do consistency check on backup: " + e.getMessage(), e, 2 ); } } diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java index 61dcb93deacda..c07288a5ec701 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java @@ -19,6 +19,9 @@ */ package org.neo4j.backup; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -45,6 +48,7 @@ import org.neo4j.kernel.configuration.Config; import org.neo4j.test.rule.TestDirectory; +import static java.lang.String.format; import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -168,6 +172,7 @@ public void nonExistingBackupDirThrows() throws CommandFailed, IncorrectUsage, B final String path = path( "/Idontexist/sasdfasdfa" ); expected.expect( CommandFailed.class ); expected.expectMessage( "Directory '" + path + "' does not exist." ); + expected.expect( exitCode( 1 ) ); execute( backupDir( path ), "--name=mybackup" ); } @@ -238,17 +243,33 @@ public void shouldNotAskForConsistencyCheckIfSpecifiedIncremental() throws Excep } @Test - public void failedCCIsReported() throws Exception + public void inconsistentCCIsReportedWithExitCode3() throws Exception { final Path path = Paths.get( "/foo/bar" ); when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(), - anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) ).thenReturn( - ConsistencyCheckService.Result.failure( path.toFile() ) ); + anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) ) + .thenReturn( + ConsistencyCheckService.Result.failure( path.toFile() ) ); expected.expect( CommandFailed.class ); expected.expectMessage( "Inconsistencies found. See '" + path + "' for details." ); + expected.expect( exitCode( 3 ) ); + execute( "--check-consistency=true", backupDir(), "--name=mybackup" ); + } + + @Test + public void errorInCCIsReportedWithExitCode2() throws Exception + + { + when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(), + anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) ) + .thenThrow( new RuntimeException( "craassh" ) ); + + expected.expect( CommandFailed.class ); + expected.expectMessage( "Failed to do consistency check on backup: craassh" ); + expected.expect( exitCode( 2 ) ); execute( "--check-consistency=true", backupDir(), "--name=mybackup" ); } @@ -317,7 +338,7 @@ public void shouldFallbackToFullIfIncrementalFails() throws Exception when( outsideWorld.fileSystem() ).thenReturn( mockFs ); File dir = testDirectory.directory( "ccInc" ); when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true ); - when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} ); + when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } ); when( backupService.doIncrementalBackup( any(), anyInt(), any(), anyLong(), any() ) ) .thenThrow( new RuntimeException( "nah-ah" ) ); @@ -341,7 +362,7 @@ public void failToRenameIsReported() throws Exception when( outsideWorld.fileSystem() ).thenReturn( mockFs ); File dir = testDirectory.directory( "ccInc" ); when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true ); - when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} ); + when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } ); when( backupService.doIncrementalBackup( any(), anyInt(), any(), anyLong(), any() ) ) .thenThrow( new RuntimeException( "nah-ah" ) ); @@ -349,11 +370,49 @@ public void failToRenameIsReported() throws Exception expected.expectMessage( "Failed to move old backup out of the way: kaboom" ); expected.expect( CommandFailed.class ); + expected.expect( exitCode( 1 ) ); execute( "--cc-report-dir=" + dir.getParent(), backupDir( dir.getParent() ), "--name=" + dir.getName() ); } + private Matcher exitCode( final int expectedCode ) + { + return new BaseMatcher() + { + @Override + public void describeTo( Description description ) + { + description.appendText( "expected exit code " ).appendValue( expectedCode ); + } + + @Override + public void describeMismatch( Object o, Description description ) + { + if ( o instanceof CommandFailed ) + { + CommandFailed e = (CommandFailed) o; + description.appendText( "but it was " ).appendValue( e.code() ); + } + else + { + description.appendText( "but exception is not a CommandFailed exception" ); + } + } + + @Override + public boolean matches( Object o ) + { + if ( o instanceof CommandFailed ) + { + CommandFailed e = (CommandFailed) o; + return expectedCode == e.code(); + } + return false; + } + }; + } + @Test public void shouldNotFallbackToFullIfSpecified() throws Exception @@ -365,6 +424,7 @@ public void shouldNotFallbackToFullIfSpecified() throws Exception expected.expectMessage( "Backup failed: nah-ah" ); expected.expect( CommandFailed.class ); + expected.expect( exitCode( 1 ) ); execute( "--fallback-to-full=false", backupDir( dir.getParent() ), "--name=" + dir.getName() ); } @@ -376,7 +436,7 @@ public void renamingOldBackupIncrements() throws Exception File dir = testDirectory.directory( "ccInc" ); when( outsideWorld.fileSystem() ).thenReturn( mockFs ); when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true ); - when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} ); + when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } ); for ( int i = 1; i < 50; i++ ) { when( mockFs.fileExists( eq( new File( dir.getParentFile(), "ccInc.err." + i ) ) ) ).thenReturn( true ); @@ -404,7 +464,7 @@ public void renamingOldBackupIncrementsOnlySoFar() throws Exception File dir = testDirectory.directory( "ccInc" ); when( outsideWorld.fileSystem() ).thenReturn( mockFs ); when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true ); - when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} ); + when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } ); for ( int i = 1; i < MAX_OLD_BACKUPS; i++ ) { when( mockFs.fileExists( eq( new File( dir.getParentFile(), "ccInc.err." + i ) ) ) ).thenReturn( true ); @@ -414,6 +474,7 @@ public void renamingOldBackupIncrementsOnlySoFar() throws Exception expected.expect( CommandFailed.class ); expected.expectMessage( "ailed to move old backup out of the way: too many old backups." ); + expected.expect( exitCode( 1 ) ); execute( "--cc-report-dir=" + dir.getParent(), backupDir( dir.getParent() ), "--name=" + dir.getName() ); } @@ -422,7 +483,7 @@ public void shouldSpecifyReportDirIfSpecified() throws Exception { File reportDir = testDirectory.directory( "ccreport" ); when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(), anyBoolean(), - any(CheckConsistencyConfig.class) ) ).thenReturn( ConsistencyCheckService.Result.success( null ) ); + any( CheckConsistencyConfig.class ) ) ).thenReturn( ConsistencyCheckService.Result.success( null ) ); execute( "--check-consistency", backupDir(), "--name=mybackup", "--cc-report-dir=" + reportDir ); @@ -443,6 +504,7 @@ public void fullFailureIsReported() throws Exception expected.expect( CommandFailed.class ); expected.expectMessage( "Backup failed: nope" ); + expected.expect( exitCode( 1 ) ); execute( backupDir( dir.getParent() ), "--name=" + dir.getName() ); } @@ -461,6 +523,7 @@ public void reportDirMustExist() throws Exception final String path = path( "/aalivnmoimzlckmvPDK" ); expected.expect( CommandFailed.class ); expected.expectMessage( "Directory '" + path + "' does not exist." ); + expected.expect( exitCode( 1 ) ); execute( "--check-consistency", backupDir(), "--name=mybackup", "--cc-report-dir=" + path ); } @@ -539,7 +602,7 @@ public void shouldPrintNiceHelp() throws Throwable usage.printUsageForCommand( new OnlineBackupCommandProvider(), ps::println ); assertEquals( - String.format( "usage: neo4j-admin backup --backup-dir= --name=%n" + + format( "usage: neo4j-admin backup --backup-dir= --name=%n" + " [--from=
] [--fallback-to-full[=]]%n" + " [--timeout=]%n" + " [--check-consistency[=]]%n" + diff --git a/enterprise/causal-clustering/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java b/enterprise/causal-clustering/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java index ea2add29a5480..4c4115f9c043d 100644 --- a/enterprise/causal-clustering/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java +++ b/enterprise/causal-clustering/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java @@ -103,13 +103,9 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed { throw new IncorrectUsage( e.getMessage() ); } - catch ( UnbindFailureException | CannotWriteException | IOException e ) + catch ( UnbindFailureException | CannotWriteException | IOException | ClusterStateException e ) { - throw new CommandFailed( "Unbind failed: " + e.getMessage(), e ); - } - catch ( ClusterStateException e ) - { - throw new CommandFailed( e ); + throw new CommandFailed( e.getMessage(), e ); } }