Improved error handling for dump command

community/dbms/src/main/java/org/neo4j/commandline/dbms/DumpCommand.java

@@ -20,6 +20,8 @@
 package org.neo4j.commandline.dbms;
 
 import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Optional;
@@ -35,10 +37,12 @@
 import org.neo4j.helpers.Args;
 import org.neo4j.server.configuration.ConfigLoader;
 
+import static java.lang.String.format;
 import static java.util.Arrays.asList;
 
 import static org.neo4j.dbms.DatabaseManagementSystemSettings.database_path;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
+import static org.neo4j.kernel.impl.util.Converters.identity;
 import static org.neo4j.kernel.impl.util.Converters.mandatory;
 
 public class DumpCommand implements AdminCommand
@@ -83,15 +87,28 @@ public DumpCommand( Path homeDir, Path configDir, Dumper dumper )
     @Override
     public void execute( String[] args ) throws IncorrectUsage, CommandFailed
     {
-        Path databaseDirectory = parse( args, "database", this::toDatabaseDirectory );
+        String database = parse( args, "database", identity() );
+        Path databaseDirectory = toDatabaseDirectory( database );
         Path archive = parse( args, "to", Paths::get );
         try
         {
             dumper.dump( databaseDirectory, archive );
         }
+        catch ( FileAlreadyExistsException e )
+        {
+            throw new CommandFailed( "archive already exists: " + e.getMessage(), e );
+        }
+        catch ( NoSuchFileException e )
+        {
+            if ( databaseDirectory.toString().equals( e.getMessage() ) )
+            {
+                throw new CommandFailed( "database does not exist: " + database, e );
+            }
+            wrapIOException( e );
+        }
         catch ( IOException e )
         {
-            throw new CommandFailed( "unable to dump database", e );
+            wrapIOException( e );
         }
     }
 
@@ -110,11 +127,17 @@ private <T> T parse( String[] args, String argument, Function<String, T> convert
     private Path toDatabaseDirectory( String databaseName )
     {
         //noinspection unchecked
-        return new ConfigLoader( asList( DatabaseManagementSystemSettings.class, GraphDatabaseSettings.class     ) )
+        return new ConfigLoader( asList( DatabaseManagementSystemSettings.class, GraphDatabaseSettings.class ) )
                 .loadConfig(
                         Optional.of( homeDir.toFile() ),
                         Optional.of( configDir.resolve( "neo4j.conf" ).toFile() ) )
                 .with( stringMap( DatabaseManagementSystemSettings.active_database.name(), databaseName ) )
                 .get( database_path ).toPath();
     }
+
+    private void wrapIOException( IOException e ) throws CommandFailed
+    {
+        throw new CommandFailed(
+                format( "unable to dump database: %s: %s", e.getClass().getSimpleName(), e.getMessage() ), e );
+    }
 }

community/dbms/src/test/java/org/neo4j/commandline/dbms/DumpCommandTest.java

@@ -20,7 +20,9 @@
 package org.neo4j.commandline.dbms;
 
 import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
@@ -108,16 +110,66 @@ public void shouldObjectIfTheArchiveArgumentIsMissing() throws CommandFailed
     }
 
     @Test
-    public void shouldThrowIfTheCommandFails() throws IOException, IncorrectUsage
+    public void shouldGiveAClearErrorIfTheArchiveAlreadyExists() throws IOException, IncorrectUsage
     {
-        doThrow( IOException.class ).when( dumper ).dump( any(), any() );
+        doThrow( new FileAlreadyExistsException( "the-archive-path" ) ).when( dumper ).dump( any(), any() );
         try
         {
             execute( null );
             fail( "expected exception" );
         }
-        catch ( CommandFailed ignored )
+        catch ( CommandFailed e )
         {
+            assertThat( e.getMessage(), equalTo( "archive already exists: the-archive-path" ) );
+        }
+    }
+
+    @Test
+    public void shouldGiveAClearMessageIfTheDatabaseDoesntExist() throws IOException, IncorrectUsage
+    {
+        doThrow( new NoSuchFileException( homeDir.resolve( "data/databases/foo.db" ).toString() ) )
+                .when( dumper ).dump( any(), any() );
+        try
+        {
+            execute( "foo.db" );
+            fail( "expected exception" );
+        }
+        catch ( CommandFailed e )
+        {
+            assertThat( e.getMessage(), equalTo( "database does not exist: foo.db" ) );
+        }
+    }
+
+    @Test
+    public void shouldGiveAClearMessageIfTheArchivesParentDoesntExist() throws IOException, IncorrectUsage
+    {
+        doThrow( new NoSuchFileException( archive.getParent().toString() ) ).when( dumper ).dump( any(), any() );
+        try
+        {
+            execute( "foo.db" );
+            fail( "expected exception" );
+        }
+        catch ( CommandFailed e )
+        {
+            assertThat( e.getMessage(),
+                    equalTo( "unable to dump database: NoSuchFileException: " + archive.getParent() ) );
+        }
+    }
+
+    @Test
+    public void
+    shouldWrapIOExceptionsCarefulllyBecauseCriticalInformationIsOftenEncodedInTheirNameButMissingFromTheirMessage()
+            throws IOException, IncorrectUsage
+    {
+        doThrow( new IOException( "the-message" ) ).when( dumper ).dump( any(), any() );
+        try
+        {
+            execute( null );
+            fail( "expected exception" );
+        }
+        catch ( CommandFailed e )
+        {
+            assertThat( e.getMessage(), equalTo( "unable to dump database: IOException: the-message" ) );
         }
     }
 

community/kernel/src/main/java/org/neo4j/kernel/impl/util/Converters.java

@@ -57,6 +57,11 @@ public static Function<String, Path> toPath()
         return Paths::get;
     }
 
+    public static Function<String, String> identity()
+    {
+        return s -> s;
+    }
+
     public static final Comparator<File> BY_FILE_NAME = ( o1, o2 ) -> o1.getName().compareTo( o2.getName() );
 
     public static final Comparator<File> BY_FILE_NAME_WITH_CLEVER_NUMBERS =