diff --git a/README.md b/README.md index a5c8355..b1b48f7 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ CBZOptimizer is a Go-based tool designed to optimize CBZ (Comic Book Zip) and CB - Process multiple chapters in parallel. - Option to override the original files (CBR files are converted to CBZ and original CBR is deleted). - Watch a folder for new CBZ/CBR files and optimize them automatically. +- Set time limits for chapter conversion to avoid hanging on problematic files. ## Installation @@ -41,6 +42,12 @@ Optimize all CBZ/CBR files in a folder recursively: cbzconverter optimize [folder] --quality 85 --parallelism 2 --override --format webp --split ``` +With timeout to avoid hanging on problematic chapters: + +```sh +cbzconverter optimize [folder] --timeout 10m --quality 85 +``` + Or with Docker: ```sh @@ -68,6 +75,7 @@ docker run -v /path/to/comics:/comics ghcr.io/belphemur/cbzoptimizer:latest watc - `--override`, `-o`: Override the original files. For CBZ files, overwrites the original. For CBR files, deletes the original CBR and creates a new CBZ. Default is false. - `--split`, `-s`: Split long pages into smaller chunks. Default is false. - `--format`, `-f`: Format to convert the images to (e.g., webp). Default is webp. +- `--timeout`, `-t`: Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout. Default is 0. - `--log`, `-l`: Set log level; can be 'panic', 'fatal', 'error', 'warn', 'info', 'debug', or 'trace'. Default is info. ## Logging diff --git a/cmd/cbzoptimizer/commands/optimize_command.go b/cmd/cbzoptimizer/commands/optimize_command.go index 8c34624..ed7cc68 100644 --- a/cmd/cbzoptimizer/commands/optimize_command.go +++ b/cmd/cbzoptimizer/commands/optimize_command.go @@ -32,6 +32,7 @@ func init() { command.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel") command.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files") command.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") + command.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout") command.PersistentFlags().VarP( formatFlag, "format", "f", @@ -80,6 +81,13 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error { } log.Debug().Bool("split", split).Msg("Split parameter parsed") + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + log.Error().Err(err).Msg("Failed to parse timeout flag") + return fmt.Errorf("invalid timeout value") + } + log.Debug().Dur("timeout", timeout).Msg("Timeout parameter parsed") + parallelism, err := cmd.Flags().GetInt("parallelism") if err != nil || parallelism < 1 { log.Error().Err(err).Int("parallelism", parallelism).Msg("Invalid parallelism value") @@ -126,6 +134,7 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error { Quality: quality, Override: override, Split: split, + Timeout: timeout, }) if err != nil { log.Error().Int("worker_id", workerID).Str("file_path", path).Err(err).Msg("Worker encountered error") diff --git a/cmd/cbzoptimizer/commands/optimize_command_test.go b/cmd/cbzoptimizer/commands/optimize_command_test.go index ed27b02..a291686 100644 --- a/cmd/cbzoptimizer/commands/optimize_command_test.go +++ b/cmd/cbzoptimizer/commands/optimize_command_test.go @@ -1,24 +1,26 @@ package commands import ( - "github.com/belphemur/CBZOptimizer/v2/internal/cbz" - "github.com/belphemur/CBZOptimizer/v2/internal/manga" - "github.com/belphemur/CBZOptimizer/v2/internal/utils/errs" - "github.com/belphemur/CBZOptimizer/v2/pkg/converter" - "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" - "github.com/spf13/cobra" + "context" "log" "os" "path/filepath" "strings" "testing" "time" + + "github.com/belphemur/CBZOptimizer/v2/internal/cbz" + "github.com/belphemur/CBZOptimizer/v2/internal/manga" + "github.com/belphemur/CBZOptimizer/v2/internal/utils/errs" + "github.com/belphemur/CBZOptimizer/v2/pkg/converter" + "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" + "github.com/spf13/cobra" ) // MockConverter is a mock implementation of the Converter interface type MockConverter struct{} -func (m *MockConverter) ConvertChapter(chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { +func (m *MockConverter) ConvertChapter(ctx context.Context, chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { chapter.IsConverted = true chapter.ConvertedTime = time.Now() return chapter, nil diff --git a/cmd/cbzoptimizer/commands/watch_command.go b/cmd/cbzoptimizer/commands/watch_command.go index 56242bd..5a9105b 100644 --- a/cmd/cbzoptimizer/commands/watch_command.go +++ b/cmd/cbzoptimizer/commands/watch_command.go @@ -39,6 +39,9 @@ func init() { command.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks") _ = viper.BindPFlag("split", command.Flags().Lookup("split")) + command.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout") + _ = viper.BindPFlag("timeout", command.Flags().Lookup("timeout")) + command.PersistentFlags().VarP( formatFlag, "format", "f", @@ -67,6 +70,8 @@ func WatchCommand(_ *cobra.Command, args []string) error { split := viper.GetBool("split") + timeout := viper.GetDuration("timeout") + converterType := constant.FindConversionFormat(viper.GetString("format")) chapterConverter, err := converter.Get(converterType) if err != nil { @@ -122,6 +127,7 @@ func WatchCommand(_ *cobra.Command, args []string) error { Quality: quality, Override: override, Split: split, + Timeout: timeout, }) if err != nil { errors <- fmt.Errorf("error processing file %s: %w", event.Filename, err) diff --git a/internal/utils/optimize.go b/internal/utils/optimize.go index 4dcad81..6a8b912 100644 --- a/internal/utils/optimize.go +++ b/internal/utils/optimize.go @@ -1,11 +1,13 @@ package utils import ( + "context" "errors" "fmt" "os" "path/filepath" "strings" + "time" "github.com/belphemur/CBZOptimizer/v2/internal/cbz" "github.com/belphemur/CBZOptimizer/v2/pkg/converter" @@ -19,6 +21,7 @@ type OptimizeOptions struct { Quality uint8 Override bool Split bool + Timeout time.Duration } // Optimize optimizes a CBZ/CBR file using the specified converter. @@ -57,7 +60,17 @@ func Optimize(options *OptimizeOptions) error { Bool("split", options.Split). Msg("Starting chapter conversion") - convertedChapter, err := options.ChapterConverter.ConvertChapter(chapter, options.Quality, options.Split, func(msg string, current uint32, total uint32) { + var ctx context.Context + if options.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.Background(), options.Timeout) + defer cancel() + log.Debug().Str("file", chapter.FilePath).Dur("timeout", options.Timeout).Msg("Applying timeout to chapter conversion") + } else { + ctx = context.Background() + } + + convertedChapter, err := options.ChapterConverter.ConvertChapter(ctx, chapter, options.Quality, options.Split, func(msg string, current uint32, total uint32) { if current%10 == 0 || current == total { log.Info().Str("file", chapter.FilePath).Uint32("current", current).Uint32("total", total).Msg("Converting") } else { diff --git a/internal/utils/optimize_test.go b/internal/utils/optimize_test.go index 56a6326..ebcf37d 100644 --- a/internal/utils/optimize_test.go +++ b/internal/utils/optimize_test.go @@ -1,6 +1,8 @@ package utils import ( + "context" + "fmt" "os" "path/filepath" "strings" @@ -18,11 +20,32 @@ type MockConverter struct { shouldFail bool } -func (m *MockConverter) ConvertChapter(chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { +func (m *MockConverter) ConvertChapter(ctx context.Context, chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { if m.shouldFail { return nil, &MockError{message: "mock conversion error"} } + // Check if context is already cancelled + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + + // Simulate some work that can be interrupted by context cancellation + for i := 0; i < len(chapter.Pages); i++ { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + // Simulate processing time + time.Sleep(100 * time.Microsecond) + if progress != nil { + progress(fmt.Sprintf("Converting page %d/%d", i+1, len(chapter.Pages)), uint32(i+1), uint32(len(chapter.Pages))) + } + } + } + // Create a copy of the chapter to simulate conversion converted := &manga.Chapter{ FilePath: chapter.FilePath, @@ -192,6 +215,7 @@ func TestOptimize(t *testing.T) { Quality: 85, Override: tt.override, Split: false, + Timeout: 0, } // Run optimization @@ -305,6 +329,7 @@ func TestOptimize_AlreadyConverted(t *testing.T) { Quality: 85, Override: false, Split: false, + Timeout: 0, } err = Optimize(options) @@ -326,6 +351,7 @@ func TestOptimize_InvalidFile(t *testing.T) { Quality: 85, Override: false, Split: false, + Timeout: 0, } err := Optimize(options) @@ -333,3 +359,66 @@ func TestOptimize_InvalidFile(t *testing.T) { t.Error("Expected error for nonexistent file") } } + +func TestOptimize_Timeout(t *testing.T) { + // Create temporary directory + tempDir, err := os.MkdirTemp("", "test_optimize_timeout") + if err != nil { + t.Fatal(err) + } + defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory") + + // Copy test files + testdataDir := "../../testdata" + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skip("testdata directory not found, skipping tests") + } + + var cbzFile string + err = filepath.Walk(testdataDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() && strings.HasSuffix(strings.ToLower(info.Name()), ".cbz") && !strings.Contains(info.Name(), "converted") { + destPath := filepath.Join(tempDir, "test.cbz") + data, err := os.ReadFile(path) + if err != nil { + return err + } + err = os.WriteFile(destPath, data, info.Mode()) + if err != nil { + return err + } + cbzFile = destPath + return filepath.SkipDir + } + return nil + }) + if err != nil { + t.Fatal(err) + } + + if cbzFile == "" { + t.Skip("No CBZ test file found") + } + + // Test with short timeout (500 microseconds) to force timeout during conversion + options := &OptimizeOptions{ + ChapterConverter: &MockConverter{}, + Path: cbzFile, + Quality: 85, + Override: false, + Split: false, + Timeout: 500 * time.Microsecond, // 500 microseconds - should timeout during page processing + } + + err = Optimize(options) + if err == nil { + t.Error("Expected timeout error but got none") + } + + // Check that the error contains timeout information + if !strings.Contains(err.Error(), "context deadline exceeded") { + t.Errorf("Expected timeout error message, got: %v", err) + } +} diff --git a/pkg/converter/converter.go b/pkg/converter/converter.go index 62719d2..e7a59b1 100644 --- a/pkg/converter/converter.go +++ b/pkg/converter/converter.go @@ -1,12 +1,14 @@ package converter import ( + "context" "fmt" + "strings" + "github.com/belphemur/CBZOptimizer/v2/internal/manga" "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" "github.com/belphemur/CBZOptimizer/v2/pkg/converter/webp" "github.com/samber/lo" - "strings" ) type Converter interface { @@ -15,7 +17,7 @@ type Converter interface { // ConvertChapter converts a manga chapter to the specified format. // // Returns partial success where some pages are converted and some are not. - ConvertChapter(chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) + ConvertChapter(ctx context.Context, chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) PrepareConverter() error } diff --git a/pkg/converter/converter_test.go b/pkg/converter/converter_test.go index 525a7c4..6563f86 100644 --- a/pkg/converter/converter_test.go +++ b/pkg/converter/converter_test.go @@ -2,14 +2,16 @@ package converter import ( "bytes" - "github.com/belphemur/CBZOptimizer/v2/internal/manga" - "github.com/belphemur/CBZOptimizer/v2/internal/utils/errs" - "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" - "golang.org/x/exp/slices" + "context" "image" "image/jpeg" "os" "testing" + + "github.com/belphemur/CBZOptimizer/v2/internal/manga" + "github.com/belphemur/CBZOptimizer/v2/internal/utils/errs" + "github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant" + "golang.org/x/exp/slices" ) func TestConvertChapter(t *testing.T) { @@ -83,7 +85,7 @@ func TestConvertChapter(t *testing.T) { t.Log(msg) } - convertedChapter, err := converter.ConvertChapter(chapter, quality, tc.split, progress) + convertedChapter, err := converter.ConvertChapter(context.Background(), chapter, quality, tc.split, progress) if err != nil { if convertedChapter != nil && slices.Contains(tc.expectPartialSuccess, converter.Format()) { t.Logf("Partial success to convert genTestChapter: %v", err) diff --git a/pkg/converter/webp/webp_converter.go b/pkg/converter/webp/webp_converter.go index e2701f5..174fdd5 100644 --- a/pkg/converter/webp/webp_converter.go +++ b/pkg/converter/webp/webp_converter.go @@ -2,6 +2,7 @@ package webp import ( "bytes" + "context" "errors" "fmt" "image" @@ -53,7 +54,7 @@ func (converter *Converter) PrepareConverter() error { return nil } -func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { +func (converter *Converter) ConvertChapter(ctx context.Context, chapter *manga.Chapter, quality uint8, split bool, progress func(message string, current uint32, total uint32)) (*manga.Chapter, error) { log.Debug(). Str("chapter", chapter.FilePath). Int("pages", len(chapter.Pages)). @@ -89,26 +90,55 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8 Int("worker_count", maxGoroutines). Msg("Initialized conversion worker pool") + // Check if context is already cancelled + select { + case <-ctx.Done(): + log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") + return nil, ctx.Err() + default: + } + // Start the worker pool go func() { + defer close(doneChan) for page := range pagesChan { - guard <- struct{}{} // would block if guard channel is already filled + select { + case <-ctx.Done(): + return + case guard <- struct{}{}: // would block if guard channel is already filled + } + go func(pageToConvert *manga.PageContainer) { defer func() { wgConvertedPages.Done() <-guard }() + // Check context cancellation before processing + select { + case <-ctx.Done(): + return + default: + } + convertedPage, err := converter.convertPage(pageToConvert, quality) if err != nil { if convertedPage == nil { - errChan <- err + select { + case errChan <- err: + case <-ctx.Done(): + return + } return } buffer := new(bytes.Buffer) err := png.Encode(buffer, convertedPage.Image) if err != nil { - errChan <- err + select { + case errChan <- err: + case <-ctx.Done(): + return + } return } convertedPage.Page.Contents = buffer @@ -121,45 +151,77 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8 pagesMutex.Unlock() }(page) } - close(doneChan) }() // Process pages for _, page := range chapter.Pages { + select { + case <-ctx.Done(): + log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") + return nil, ctx.Err() + default: + } + go func(page *manga.Page) { defer wgPages.Done() splitNeeded, img, format, err := converter.checkPageNeedsSplit(page, split) if err != nil { - errChan <- err + select { + case errChan <- err: + case <-ctx.Done(): + return + } if img != nil { wgConvertedPages.Add(1) - pagesChan <- manga.NewContainer(page, img, format, false) + select { + case pagesChan <- manga.NewContainer(page, img, format, false): + case <-ctx.Done(): + return + } } return } if !splitNeeded { wgConvertedPages.Add(1) - pagesChan <- manga.NewContainer(page, img, format, true) + select { + case pagesChan <- manga.NewContainer(page, img, format, true): + case <-ctx.Done(): + return + } return } images, err := converter.cropImage(img) if err != nil { - errChan <- err + select { + case errChan <- err: + case <-ctx.Done(): + return + } return } atomic.AddUint32(&totalPages, uint32(len(images)-1)) for i, img := range images { + select { + case <-ctx.Done(): + return + default: + } + newPage := &manga.Page{ Index: page.Index, IsSplitted: true, SplitPartIndex: uint16(i), } wgConvertedPages.Add(1) - pagesChan <- manga.NewContainer(newPage, img, "N/A", true) + select { + case pagesChan <- manga.NewContainer(newPage, img, "N/A", true): + case <-ctx.Done(): + return + } } }(page) } @@ -167,9 +229,21 @@ func (converter *Converter) ConvertChapter(chapter *manga.Chapter, quality uint8 wgPages.Wait() close(pagesChan) - // Wait for all conversions to complete - <-doneChan - wgConvertedPages.Wait() + // Wait for all conversions to complete or context cancellation + done := make(chan struct{}) + go func() { + defer close(done) + wgConvertedPages.Wait() + }() + + select { + case <-done: + // Conversion completed successfully + case <-ctx.Done(): + log.Warn().Str("chapter", chapter.FilePath).Msg("Chapter conversion cancelled due to timeout") + return nil, ctx.Err() + } + close(errChan) close(guard) diff --git a/pkg/converter/webp/webp_converter_test.go b/pkg/converter/webp/webp_converter_test.go index 13f84bf..30329ba 100644 --- a/pkg/converter/webp/webp_converter_test.go +++ b/pkg/converter/webp/webp_converter_test.go @@ -2,12 +2,14 @@ package webp import ( "bytes" + "context" "image" "image/color" + "image/jpeg" "image/png" "sync" "testing" - "image/jpeg" + _ "golang.org/x/image/webp" "github.com/belphemur/CBZOptimizer/v2/internal/manga" @@ -170,7 +172,7 @@ func TestConverter_ConvertChapter(t *testing.T) { assert.LessOrEqual(t, current, total, "Current progress should not exceed total") } - convertedChapter, err := converter.ConvertChapter(chapter, 80, tt.split, progress) + convertedChapter, err := converter.ConvertChapter(context.Background(), chapter, 80, tt.split, progress) if tt.expectError { assert.Error(t, err) @@ -322,3 +324,42 @@ func TestConverter_Format(t *testing.T) { converter := New() assert.Equal(t, constant.WebP, converter.Format()) } + +func TestConverter_ConvertChapter_Timeout(t *testing.T) { + converter := New() + err := converter.PrepareConverter() + require.NoError(t, err) + + // Create a test chapter with a few pages + pages := []*manga.Page{ + createTestPage(t, 1, 800, 1200, "jpeg"), + createTestPage(t, 2, 800, 1200, "jpeg"), + createTestPage(t, 3, 800, 1200, "jpeg"), + } + + chapter := &manga.Chapter{ + FilePath: "/test/chapter.cbz", + Pages: pages, + } + + var progressMutex sync.Mutex + var lastProgress uint32 + progress := func(message string, current uint32, total uint32) { + progressMutex.Lock() + defer progressMutex.Unlock() + assert.GreaterOrEqual(t, current, lastProgress, "Progress should never decrease") + lastProgress = current + assert.LessOrEqual(t, current, total, "Current progress should not exceed total") + } + + // Test with very short timeout (1 nanosecond) + ctx, cancel := context.WithTimeout(context.Background(), 1) + defer cancel() + + convertedChapter, err := converter.ConvertChapter(ctx, chapter, 80, false, progress) + + // Should return context error due to timeout + assert.Error(t, err) + assert.Nil(t, convertedChapter) + assert.Equal(t, context.DeadlineExceeded, err) +}