From 52ceb192e26f766d8b830ab220051db00a7862a7 Mon Sep 17 00:00:00 2001 From: b00f Date: Thu, 25 Jul 2024 16:23:53 +0800 Subject: [PATCH] fix(cmd): prevent sudden crash on download error (#1432) --- cmd/daemon/import.go | 7 +-- cmd/gtk/startup_assistant.go | 10 ++-- cmd/importer.go | 39 ++++++------- util/io.go | 12 +++- util/io_test.go | 110 +++++++++++++++++++++++------------ 5 files changed, 108 insertions(+), 70 deletions(-) diff --git a/cmd/daemon/import.go b/cmd/daemon/import.go index c52a6cc7b..91e66d3b5 100644 --- a/cmd/daemon/import.go +++ b/cmd/daemon/import.go @@ -81,11 +81,8 @@ func buildImportCmd(parentCmd *cobra.Command) { cmd.PrintLine() - importer.Download( - c.Context(), - &selected, - downloadProgressBar, - ) + err = importer.Download(c.Context(), &selected, downloadProgressBar) + cmd.FatalErrorCheck(err) cmd.PrintLine() cmd.PrintLine() diff --git a/cmd/gtk/startup_assistant.go b/cmd/gtk/startup_assistant.go index a0d0112cc..05533097d 100644 --- a/cmd/gtk/startup_assistant.go +++ b/cmd/gtk/startup_assistant.go @@ -258,12 +258,8 @@ func startupAssistant(workingDir string, chainType genesis.ChainType) bool { go func() { log.Printf("start downloading...\n") - importer.Download( - ctx, - &md[snapshotIndex], - func(fileName string, totalSize, downloaded int64, - percentage float64, - ) { + err := importer.Download(ctx, &md[snapshotIndex], + func(fileName string, totalSize, downloaded int64, percentage float64) { percent := int(percentage) glib.IdleAdd(func() { dlMessage := fmt.Sprintf("🌐 Downloading %s | %d%% (%s / %s)", @@ -278,6 +274,8 @@ func startupAssistant(workingDir string, chainType genesis.ChainType) bool { ) glib.IdleAdd(func() { + fatalErrorCheck(err) + log.Printf("extracting data...\n") ssPBLabel.SetText(" " + "📂 Extracting downloaded files...") err := importer.ExtractAndStoreFiles() diff --git a/cmd/importer.go b/cmd/importer.go index ce8405709..a7e6b3372 100644 --- a/cmd/importer.go +++ b/cmd/importer.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "log" "net/http" "net/url" "os" @@ -23,7 +24,7 @@ const DefaultSnapshotURL = "https://snapshot.pactus.org" const maxDecompressedSize = 10 << 20 // 10 MB -type DMStateFunc func( +type ImporterStateFunc func( fileName string, totalSize, downloaded int64, percentage float64, @@ -124,47 +125,45 @@ func (i *Importer) GetMetadata(ctx context.Context) ([]Metadata, error) { return metadata, nil } -func (i *Importer) Download( - ctx context.Context, - metadata *Metadata, - stateFunc DMStateFunc, -) { - done := make(chan struct{}) +func (i *Importer) Download(ctx context.Context, metadata *Metadata, + stateFunc ImporterStateFunc, +) error { + done := make(chan error) + defer close((done)) + dlLink, err := url.JoinPath(i.snapshotURL, metadata.Data.Path) - FatalErrorCheck(err) + if err != nil { + return err + } fileName := filepath.Base(dlLink) - i.dataFileName = fileName - filePath := fmt.Sprintf("%s/%s", i.tempDir, fileName) - d := downloader.New( - dlLink, - filePath, - metadata.Data.Sha, - ) - + d := downloader.New(dlLink, filePath, metadata.Data.Sha) d.Start(ctx) go func() { err := <-d.Errors() - FatalErrorCheck(err) + if err != nil { + log.Printf("download encountered an error: %s\n", err) + done <- err + } }() go func() { for state := range d.Stats() { stateFunc(fileName, state.TotalSize, state.Downloaded, state.Percent) if state.Completed { - done <- struct{}{} - close(done) + log.Println("download completed") + done <- nil return } } }() - <-done + return <-done } func (i *Importer) Cleanup() error { diff --git a/util/io.go b/util/io.go index 9b257d997..52d931169 100644 --- a/util/io.go +++ b/util/io.go @@ -90,8 +90,7 @@ func IsDirEmpty(name string) bool { return errors.Is(err, io.EOF) } -// IsDirNotExistsOrEmpty returns true if a directory does not exist or is empty. -// It checks if the path exists and, if so, whether the directory is empty. +// IsDirNotExistsOrEmpty checks if the path exists and, if so, whether the directory is empty. func IsDirNotExistsOrEmpty(name string) bool { if !PathExists(name) { return true @@ -203,11 +202,18 @@ func NewFixedReader(max int, buf []byte) *FixedReader { // MoveDirectory moves a directory from srcDir to dstDir, including all its contents. // If dstDir already exists and is not empty, it returns an error. +// If the parent directory of dstDir does not exist, it will be created. func MoveDirectory(srcDir, dstDir string) error { - if !IsDirNotExistsOrEmpty(dstDir) { + if PathExists(dstDir) { return fmt.Errorf("destination directory %s already exists", dstDir) } + // Get the parent directory of the destination directory + parentDir := filepath.Dir(dstDir) + if err := Mkdir(parentDir); err != nil { + return fmt.Errorf("failed to create parent directories for %s: %w", dstDir, err) + } + if err := os.Rename(srcDir, dstDir); err != nil { return fmt.Errorf("failed to move directory from %s to %s: %w", srcDir, dstDir, err) } diff --git a/util/io_test.go b/util/io_test.go index aec40cd69..5d4967d79 100644 --- a/util/io_test.go +++ b/util/io_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" ) +const invalidDirName = "/invalid:path/\x00*folder?\\CON" + func TestWriteFile(t *testing.T) { p := TempDirPath() d := []byte("some-data") @@ -90,56 +92,92 @@ func TestIsValidPath(t *testing.T) { assert.False(t, IsValidDirPath("/root")) assert.False(t, IsValidDirPath("/test")) } + assert.False(t, IsValidDirPath(invalidDirName)) assert.False(t, IsValidDirPath("./io_test.go")) assert.True(t, IsValidDirPath("/tmp")) assert.True(t, IsValidDirPath("/tmp/pactus")) } func TestMoveDirectory(t *testing.T) { - // Create temporary directories - srcDir := TempDirPath() - dstDir := TempDirPath() - defer func() { _ = os.RemoveAll(srcDir) }() - defer func() { _ = os.RemoveAll(dstDir) }() - - // Create a subdirectory in the source directory - subDir := filepath.Join(srcDir, "subdir") - err := Mkdir(subDir) - assert.NoError(t, err) + t.Run("DestinationDirectoryExistsAndNotEmpty", func(t *testing.T) { + srcDir := TempDirPath() + dstDir := TempDirPath() - // Create multiple files in the subdirectory - files := []struct { - name string - content string - }{ - {"file1.txt", "content 1"}, - {"file2.txt", "content 2"}, - } + err := MoveDirectory(srcDir, dstDir) + assert.Error(t, err) + assert.Contains(t, err.Error(), "destination directory") + }) - for _, file := range files { - filePath := filepath.Join(subDir, file.name) - err = WriteFile(filePath, []byte(file.content)) + t.Run("ParentDirectoryCreationFailure", func(t *testing.T) { + srcDir := TempDirPath() + + err := MoveDirectory(srcDir, invalidDirName) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create parent directories") + }) + + t.Run("SourceDirectoryRenameFailure", func(t *testing.T) { + srcDir := TempDirPath() + dstDir := TempDirPath() + + err := os.RemoveAll(dstDir) assert.NoError(t, err) - } - // Move the directory - dstDirPath := filepath.Join(dstDir, "movedir") - err = MoveDirectory(srcDir, dstDirPath) - assert.NoError(t, err) + // Remove the source directory to simulate the rename failure + err = os.RemoveAll(srcDir) + assert.NoError(t, err) - // Assert the source directory no longer exists - assert.False(t, PathExists(srcDir)) + err = MoveDirectory(srcDir, dstDir) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to move directory") + }) - // Assert the destination directory exists - assert.True(t, PathExists(dstDirPath)) + t.Run("MoveDirectorySuccess", func(t *testing.T) { + // Create temporary directories + srcDir := TempDirPath() + dstDir := TempDirPath() + defer func() { _ = os.RemoveAll(srcDir) }() + defer func() { _ = os.RemoveAll(dstDir) }() - // Verify that all files have been moved and their contents are correct - for _, file := range files { - movedFilePath := filepath.Join(dstDirPath, "subdir", file.name) - data, err := ReadFile(movedFilePath) + // Create a subdirectory in the source directory + subDir := filepath.Join(srcDir, "subdir") + err := Mkdir(subDir) assert.NoError(t, err) - assert.Equal(t, file.content, string(data)) - } + + // Create multiple files in the subdirectory + files := []struct { + name string + content string + }{ + {"file1.txt", "content 1"}, + {"file2.txt", "content 2"}, + } + + for _, file := range files { + filePath := filepath.Join(subDir, file.name) + err = WriteFile(filePath, []byte(file.content)) + assert.NoError(t, err) + } + + // Move the directory + dstDirPath := filepath.Join(dstDir, "movedir") + err = MoveDirectory(srcDir, dstDirPath) + assert.NoError(t, err) + + // Assert the source directory no longer exists + assert.False(t, PathExists(srcDir)) + + // Assert the destination directory exists + assert.True(t, PathExists(dstDirPath)) + + // Verify that all files have been moved and their contents are correct + for _, file := range files { + movedFilePath := filepath.Join(dstDirPath, "subdir", file.name) + data, err := ReadFile(movedFilePath) + assert.NoError(t, err) + assert.Equal(t, file.content, string(data)) + } + }) } func TestSanitizeArchivePath(t *testing.T) {