diff --git a/pkg/modules/libreoffice/api/api.go b/pkg/modules/libreoffice/api/api.go index dd639100..7de4801f 100644 --- a/pkg/modules/libreoffice/api/api.go +++ b/pkg/modules/libreoffice/api/api.go @@ -28,6 +28,11 @@ var ( // ErrMalformedPageRanges happens if the page ranges option cannot be // interpreted by LibreOffice. ErrMalformedPageRanges = errors.New("page ranges are malformed") + + // ErrCoreDumped happens randomly; sometime a conversion will work as + // expected, and some other time the same conversion will fail. + // See https://github.com/gotenberg/gotenberg/issues/639. + ErrCoreDumped = errors.New("core dumped") ) // Api is a module which provides a [Uno] to interact with LibreOffice. @@ -365,9 +370,20 @@ func (a *Api) LibreOffice() (Uno, error) { // Pdf converts a document to PDF. func (a *Api) Pdf(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options Options) error { - return a.supervisor.Run(ctx, logger, func() error { + err := a.supervisor.Run(ctx, logger, func() error { return a.libreOffice.pdf(ctx, logger, inputPath, outputPath, options) }) + + if err == nil { + return nil + } + + // See https://github.com/gotenberg/gotenberg/issues/639. + if errors.Is(err, ErrCoreDumped) { + return a.Pdf(ctx, logger, inputPath, outputPath, options) + } + + return fmt.Errorf("supervisor run task: %w", err) } // Extensions returns the file extensions available for conversions. diff --git a/pkg/modules/libreoffice/api/api_test.go b/pkg/modules/libreoffice/api/api_test.go index faf6535c..b9480b85 100644 --- a/pkg/modules/libreoffice/api/api_test.go +++ b/pkg/modules/libreoffice/api/api_test.go @@ -433,6 +433,13 @@ func TestApi_Pdf(t *testing.T) { }}, expectError: true, }, + { + scenario: "ErrCoreDumped", + libreOffice: &libreOfficeMock{pdfMock: func(ctx context.Context, logger *zap.Logger, input, outputPath string, options Options) error { + return ErrCoreDumped + }}, + expectError: false, + }, } { t.Run(tc.scenario, func(t *testing.T) { a := new(Api) diff --git a/pkg/modules/libreoffice/api/libreoffice.go b/pkg/modules/libreoffice/api/libreoffice.go index de031353..afc2b705 100644 --- a/pkg/modules/libreoffice/api/libreoffice.go +++ b/pkg/modules/libreoffice/api/libreoffice.go @@ -8,6 +8,7 @@ import ( "net" "os" "path/filepath" + "strings" "sync" "sync/atomic" "time" @@ -348,6 +349,12 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP return ErrMalformedPageRanges } + // We may want to retry in case of a core dumped event. + // See https://github.com/gotenberg/gotenberg/issues/639. + if strings.Contains(err.Error(), "core dumped") { + return ErrCoreDumped + } + // Possible errors: // 1. LibreOffice failed for some reason. // 2. Context done. diff --git a/pkg/modules/libreoffice/api/mocks.go b/pkg/modules/libreoffice/api/mocks.go index 0f87d851..9c8c5c1a 100644 --- a/pkg/modules/libreoffice/api/mocks.go +++ b/pkg/modules/libreoffice/api/mocks.go @@ -2,6 +2,7 @@ package api import ( "context" + "errors" "go.uber.org/zap" @@ -33,12 +34,21 @@ func (provider *ProviderMock) LibreOffice() (Uno, error) { // libreOfficeMock is a mock for the [libreOffice] interface. type libreOfficeMock struct { + errCoreDumpedCount int + gotenberg.ProcessMock pdfMock func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options Options) error } func (b *libreOfficeMock) pdf(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options Options) error { - return b.pdfMock(ctx, logger, inputPath, outputPath, options) + err := b.pdfMock(ctx, logger, inputPath, outputPath, options) + if errors.Is(err, ErrCoreDumped) { + b.errCoreDumpedCount += 1 + } + if b.errCoreDumpedCount > 1 { + return nil + } + return err } // Interface guards. diff --git a/pkg/modules/libreoffice/api/mocks_test.go b/pkg/modules/libreoffice/api/mocks_test.go index 2c6f2e4a..f2a03530 100644 --- a/pkg/modules/libreoffice/api/mocks_test.go +++ b/pkg/modules/libreoffice/api/mocks_test.go @@ -42,14 +42,53 @@ func TestProviderMock(t *testing.T) { } func TestLibreOfficeMock(t *testing.T) { - mock := &libreOfficeMock{ - pdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options Options) error { - return nil + for _, tc := range []struct { + scenario string + mock *libreOfficeMock + expectError bool + }{ + { + scenario: "success", + mock: &libreOfficeMock{ + pdfMock: func(ctx context.Context, logger *zap.Logger, input, outputPath string, options Options) error { + return nil + }, + }, + expectError: false, }, - } + { + scenario: "ErrCoreDumped (first call)", + mock: &libreOfficeMock{ + pdfMock: func(ctx context.Context, logger *zap.Logger, input, outputPath string, options Options) error { + return ErrCoreDumped + }, + }, + expectError: true, + }, + { + scenario: "ErrCoreDumped (second call)", + mock: func() *libreOfficeMock { + m := &libreOfficeMock{ + pdfMock: func(ctx context.Context, logger *zap.Logger, input, outputPath string, options Options) error { + return ErrCoreDumped + }, + } + m.pdf(context.Background(), zap.NewNop(), "", "", Options{}) + return m + }(), + expectError: false, + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + err := tc.mock.pdf(context.Background(), zap.NewNop(), "", "", Options{}) - err := mock.pdf(context.Background(), zap.NewNop(), "", "", Options{}) - if err != nil { - t.Errorf("expected no error from libreOfficeMock.pdf, but got: %v", err) + if !tc.expectError && err != nil { + t.Fatalf("expected no error from libreOfficeMock.pdf but got: %v", err) + } + + if tc.expectError && err == nil { + t.Fatal("expected error from libreOfficeMock.pdf but got none") + } + }) } }