From 0e3b5f8a622053f38810e4c26acc9ca1dbf22c66 Mon Sep 17 00:00:00 2001 From: Mathias Grimm Date: Thu, 25 Jun 2026 06:41:06 -0300 Subject: [PATCH 1/2] Add throws() to OptimizerChain for failure handling Optimizer failures (e.g. a binary exiting non-zero) were always logged and swallowed, so callers could not observe a failure, skip the rest of the chain, or abort. Add a fluent `throws()` control on OptimizerChain: - `throws()` / `throws(true)` rethrows the failure and aborts the chain - `throws(false)` keeps the default "log and continue" behaviour - `throws($callable)` runs a custom handler receiving (Throwable $exception, Optimizer $optimizer, Image $image); return to continue, throw to abort Non-zero process exits now raise a ProcessFailedException internally that flows through the same handler. logResult() is kept intact so existing log output and the protected extension point are unchanged. With `throws()` never called the behaviour is identical to before (log and continue), so this is backwards compatible. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 37 ++++ src/OptimizerChain.php | 77 +++++++- tests/OptimizerChainErrorHandlerTest.php | 221 +++++++++++++++++++++++ 3 files changed, 328 insertions(+), 7 deletions(-) create mode 100644 tests/OptimizerChainErrorHandlerTest.php diff --git a/README.md b/README.md index ee72ac9..17288a1 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,43 @@ $optimizerChain In this example each optimizer in the chain will get a maximum 10 seconds to do it's job. +### Handling errors + +By default, when an optimizer fails (for example its binary exits with a non-zero status), the failure is logged and the chain simply continues with the next optimizer. If you'd rather be notified, or abort the whole chain, use `throws`. + +Call `throws()` to make the chain rethrow the failure and stop: + +```php +$optimizerChain + ->throws() + ->optimize($pathToImage); +``` + +Pass `false` to keep the default "log and continue" behaviour explicitly: + +```php +$optimizerChain + ->throws(false) + ->optimize($pathToImage); +``` + +Or pass a callable to decide for yourself. The handler receives the exception, the optimizer that failed and the image being optimized. Return to continue with the next optimizer, or throw to abort the chain: + +```php +use Spatie\ImageOptimizer\Image; +use Spatie\ImageOptimizer\Optimizer; + +$optimizerChain + ->throws(function (Throwable $exception, Optimizer $optimizer, Image $image) { + report($exception); + + // return to continue the chain, or throw to abort it + }) + ->optimize($pathToImage); +``` + +Any exception raised while applying an optimizer, most notably a `ProcessTimedOutException` when an optimizer exceeds its [timeout](#setting-a-timeout), flows through this same mechanism. With the default (or `throws(false)`) it is caught and the chain continues; with `throws()` or a callable you get to inspect and decide what to do with it. + ### Creating your own optimization chains If you want to customize the chain of optimizers you can do so by adding `Optimizer`s manually to an `OptimizerChain`. diff --git a/src/OptimizerChain.php b/src/OptimizerChain.php index c95b09f..7c6db9b 100644 --- a/src/OptimizerChain.php +++ b/src/OptimizerChain.php @@ -4,7 +4,9 @@ use InvalidArgumentException; use Psr\Log\LoggerInterface; +use Symfony\Component\Process\Exception\ProcessFailedException; use Symfony\Component\Process\Process; +use Throwable; class OptimizerChain { @@ -17,6 +19,9 @@ class OptimizerChain /** @var int */ protected $timeout = 60; + /** @var callable|null */ + protected $errorHandler; + public function __construct() { $this->useLogger(new DummyLogger()); @@ -62,6 +67,44 @@ public function useLogger(LoggerInterface $log) return $this; } + /* + * Control what happens when an optimizer fails. By default (when this is never called) a + * failing optimizer is logged and the chain continues. Pass true to instead rethrow and + * abort the chain, false to keep the default swallow-and-continue, or a callable that + * receives the exception, the optimizer and the image; return from it to continue or + * throw to abort. + * + * Exceptions that were previously thrown straight out of optimize() - such as a + * ProcessTimedOutException when an optimizer exceeds its timeout - now flow through here + * too, so they are caught by default and can be inspected or rethrown via this handler. + * + * This only covers failures while applying an optimizer. The "Cannot copy file" exception + * thrown by optimize() when the output file cannot be written happens before any optimizer + * runs, so it always bubbles up regardless of this setting. + */ + public function throws($handler = true) + { + if (! is_bool($handler) && ! is_callable($handler)) { + throw new InvalidArgumentException('The handler passed to `throws()` must be a boolean or a callable.'); + } + + if ($handler === false) { + $this->errorHandler = null; + + return $this; + } + + if ($handler === true) { + $handler = function (Throwable $exception) { + throw $exception; + }; + } + + $this->errorHandler = $handler; + + return $this; + } + public function optimize(string $pathToImage, ?string $pathToOutput = null) { if ($pathToOutput) { @@ -91,6 +134,22 @@ protected function applyOptimizer(Optimizer $optimizer, Image $image) $optimizer->setImagePath($image->path()); + try { + $this->runOptimizer($optimizer, $image); + } catch (Throwable $exception) { + $this->handleException($exception, $optimizer, $image); + } finally { + if ( + ($tmpPath = $optimizer->getTmpPath()) && + file_exists($tmpPath) + ) { + unlink($tmpPath); + } + } + } + + protected function runOptimizer(Optimizer $optimizer, Image $image) + { $command = $optimizer->getCommand(); $this->logger->info("Executing `{$command}`"); @@ -101,14 +160,11 @@ protected function applyOptimizer(Optimizer $optimizer, Image $image) ->setTimeout($this->timeout) ->run(); - if ( - ($tmpPath = $optimizer->getTmpPath()) && - file_exists($tmpPath) - ) { - unlink($tmpPath); - } - $this->logResult($process); + + if (! $process->isSuccessful()) { + throw new ProcessFailedException($process); + } } protected function logResult(Process $process) @@ -121,4 +177,11 @@ protected function logResult(Process $process) $this->logger->info("Process successfully ended with output `{$process->getOutput()}`"); } + + protected function handleException(Throwable $exception, Optimizer $optimizer, Image $image) + { + if ($this->errorHandler) { + ($this->errorHandler)($exception, $optimizer, $image); + } + } } diff --git a/tests/OptimizerChainErrorHandlerTest.php b/tests/OptimizerChainErrorHandlerTest.php new file mode 100644 index 0000000..b31da58 --- /dev/null +++ b/tests/OptimizerChainErrorHandlerTest.php @@ -0,0 +1,221 @@ +testImage = getTempFilePath('image.jpg'); + + $this->optimizerChain = (new OptimizerChain())->useLogger($this->log); +}); + +it('does not throw by default and keeps optimizing after a failure', function () { + $this + ->optimizerChain + ->setOptimizers([new FailingOptimizer(), new SucceedingOptimizer()]) + ->optimize($this->testImage); + + $logText = $this->log->getAllLinesAsString(); + + expect($logText) + ->toContain('Using optimizer: `FailingOptimizer`') + ->toContain('Using optimizer: `SucceedingOptimizer`') + ->toContain('error:'); +}); + +it('aborts the chain when throws() is enabled', function () { + $this + ->optimizerChain + ->throws() + ->setOptimizers([new FailingOptimizer(), new SucceedingOptimizer()]); + + expect(fn () => $this->optimizerChain->optimize($this->testImage)) + ->toThrow(ProcessFailedException::class); + + expect($this->log->getAllLinesAsString()) + ->not->toContain('Using optimizer: `SucceedingOptimizer`'); +}); + +it('keeps optimizing when throws(false) is set', function () { + $this + ->optimizerChain + ->throws(false) + ->setOptimizers([new FailingOptimizer(), new SucceedingOptimizer()]) + ->optimize($this->testImage); + + expect($this->log->getAllLinesAsString()) + ->toContain('Using optimizer: `SucceedingOptimizer`'); +}); + +it('lets a custom handler swallow the failure and continue', function () { + $captured = []; + + $this + ->optimizerChain + ->throws(function ($exception, $optimizer, $image) use (&$captured) { + $captured = [$exception, $optimizer, $image]; + }) + ->setOptimizers([new FailingOptimizer(), new SucceedingOptimizer()]) + ->optimize($this->testImage); + + expect($captured[0])->toBeInstanceOf(ProcessFailedException::class); + expect($captured[1])->toBeInstanceOf(FailingOptimizer::class); + expect($captured[2])->toBeInstanceOf(Image::class); + expect($this->log->getAllLinesAsString()) + ->toContain('Using optimizer: `SucceedingOptimizer`'); +}); + +it('lets a custom handler abort by throwing', function () { + $this + ->optimizerChain + ->throws(function () { + throw new RuntimeException('aborted'); + }) + ->setOptimizers([new FailingOptimizer(), new SucceedingOptimizer()]); + + expect(fn () => $this->optimizerChain->optimize($this->testImage)) + ->toThrow(RuntimeException::class, 'aborted'); + + expect($this->log->getAllLinesAsString()) + ->not->toContain('Using optimizer: `SucceedingOptimizer`'); +}); + +it('rejects a throws() handler that is not a boolean or callable', function () { + expect(fn () => $this->optimizerChain->throws('nope')) + ->toThrow(InvalidArgumentException::class); +}); + +it('logs the exact same lines regardless of the throws() mode', function () { + // These are the log lines the chain produced before this PR existed: a failing + // optimizer is logged via logResult() and the chain moves on. `throws()` must only + // change whether the chain continues or aborts, never what gets logged. + $expectedLines = [ + "info: Start optimizing {$this->testImage}", + 'info: Using optimizer: `FailingOptimizer`', + 'info: Executing `false`', + 'error: Process errored with ``', + ]; + + $logsFor = function ($throws) { + $logger = new ArrayLogger(); + + $chain = (new OptimizerChain()) + ->useLogger($logger) + ->setOptimizers([new FailingOptimizer()]); + + // `null` means never call throws() at all (the pre-PR / backwards-compatible default). + if ($throws !== null) { + $chain->throws($throws); + } + + try { + $chain->optimize($this->testImage); + } catch (Throwable $exception) { + // throws(true) and an aborting callback rethrow *after* logging; the log is + // what we assert on here, so swallow the abort. + } + + return $logger->getAllLines(); + }; + + expect($logsFor(null))->toBe($expectedLines); // default (pre-PR baseline) + expect($logsFor(false))->toBe($expectedLines); // throws(false) + expect($logsFor(true))->toBe($expectedLines); // throws(true) + expect($logsFor(fn () => null))->toBe($expectedLines); // callback that continues + expect($logsFor(function () { // callback that aborts + throw new RuntimeException('stop'); + }))->toBe($expectedLines); +}); + +it('passes a ProcessFailedException carrying the failed process to the handler', function () { + $captured = null; + + $this + ->optimizerChain + ->throws(function ($exception) use (&$captured) { + $captured = $exception; + }) + ->setOptimizers([new FailingOptimizer()]) + ->optimize($this->testImage); + + expect($captured)->toBeInstanceOf(ProcessFailedException::class); + expect($captured->getProcess()->isSuccessful())->toBeFalse(); +}); From 2c5ccfbfa90517d8981cbf75438ed557cbfb24fd Mon Sep 17 00:00:00 2001 From: Freek Van der Herten Date: Thu, 25 Jun 2026 23:11:12 +0200 Subject: [PATCH 2/2] Drop support for PHP 7.3 --- .github/workflows/tests.yml | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8d7875c..57b8aeb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,7 +9,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest] - php: [8.5, 8.4, 8.3, 8.2, 8.1, 8.0, 7.4, 7.3] + php: [8.5, 8.4, 8.3, 8.2, 8.1, 8.0, 7.4] dependency-version: [prefer-lowest, prefer-stable] name: P${{ matrix.php }} - ${{ matrix.dependency-version }} - ${{ matrix.os }} diff --git a/composer.json b/composer.json index c4373c7..96a1a68 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ } ], "require": { - "php": "^7.3|^8.0", + "php": "^7.4|^8.0", "ext-fileinfo": "*", "psr/log": "^1.0 | ^2.0 | ^3.0", "symfony/process": "^4.2|^5.0|^6.0|^7.0|^8.0"