diff --git a/app/Jobs/ImageOptimizePipeline/ImageResize.php b/app/Jobs/ImageOptimizePipeline/ImageResize.php index 7802fa57b..a0088c642 100644 --- a/app/Jobs/ImageOptimizePipeline/ImageResize.php +++ b/app/Jobs/ImageOptimizePipeline/ImageResize.php @@ -81,10 +81,8 @@ class ImageResize implements ShouldQueue try { $img = new Image; $img->resizeImage($media); - } catch (\Exception $e) { - if (config('app.dev_log')) { - Log::error('Image resize failed: '.$e->getMessage()); - } + } catch (\Throwable $e) { + Log::error("ImageResize: media {$media->id} was not resized [".$e::class.']: '.$e->getMessage()); } ImageThumbnail::dispatch($media)->onQueue('mmo'); diff --git a/app/Jobs/ImageOptimizePipeline/ImageThumbnail.php b/app/Jobs/ImageOptimizePipeline/ImageThumbnail.php index 12b5efe76..7d0f2f1c9 100644 --- a/app/Jobs/ImageOptimizePipeline/ImageThumbnail.php +++ b/app/Jobs/ImageOptimizePipeline/ImageThumbnail.php @@ -64,12 +64,11 @@ class ImageThumbnail implements ShouldQueue try { $img = new Image; $img->resizeThumbnail($media); - } catch (\Exception $e) { - if (config('app.dev_log')) { - Log::error('Thumbnail generation failed: '.$e->getMessage()); - } - - return; + } catch (\Throwable $e) { + // Keep going: returning here left the media without processed_at and + // never dispatched ImageUpdate, so it never reached cloud storage and + // its status never federated. + Log::error("ImageThumbnail: media {$media->id} has no thumbnail [".$e::class.']: '.$e->getMessage()); } $media->processed_at = now(); diff --git a/app/Jobs/ImageOptimizePipeline/ImageUpdate.php b/app/Jobs/ImageOptimizePipeline/ImageUpdate.php index 589502f34..44116bcdb 100644 --- a/app/Jobs/ImageOptimizePipeline/ImageUpdate.php +++ b/app/Jobs/ImageOptimizePipeline/ImageUpdate.php @@ -22,6 +22,7 @@ class ImageUpdate implements ShouldQueue protected $protectedMimes = [ 'image/jpeg', + 'image/jpg', 'image/png', 'image/webp', 'image/avif', diff --git a/app/Util/Media/Image.php b/app/Util/Media/Image.php index 8cc6dbc8d..6e412bef0 100644 --- a/app/Util/Media/Image.php +++ b/app/Util/Media/Image.php @@ -306,10 +306,18 @@ class Image StatusService::del($media->status_id); } - } catch (\Exception $e) { - if (config('app.dev_log')) { - Log::info('MediaResizeException: '.$e->getMessage().' | Could not process media id: '.$media->id); - } + } catch (\Throwable $e) { + // Always logged, never gated behind dev_log: when this fails the + // untouched original upload keeps being served at full size, and + // nothing else in the app surfaces that. + Log::error(sprintf( + 'MediaResizeException: could not %s media id %s (%s) [%s]: %s', + $thumbnail ? 'thumbnail' : 'resize', + $media->id, + $media->mime, + $e::class, + $e->getMessage() + )); } } diff --git a/app/Util/Media/ImageDriverManager.php b/app/Util/Media/ImageDriverManager.php index d94b590d2..0e8378d3a 100644 --- a/app/Util/Media/ImageDriverManager.php +++ b/app/Util/Media/ImageDriverManager.php @@ -2,11 +2,22 @@ namespace App\Util\Media; +use Illuminate\Support\Facades\Log; use Intervention\Image\Drivers\Gd\Driver; use Intervention\Image\ImageManager; +use Throwable; class ImageDriverManager { + /** + * Configured driver name => driver name that passed its health check in + * this process. Driver construction runs a health check (the vips driver + * boots FFI + libvips), so the result is memoized per worker. + * + * @var array + */ + protected static array $resolved = []; + /** * Get the appropriate image driver class based on configuration. * @@ -23,17 +34,93 @@ class ImageDriverManager } /** - * Create a new ImageManager instance with the configured driver. + * Create a new ImageManager instance. + * + * With an explicit $driver the call is strict and throws when that driver + * is unavailable. Without one, the configured driver is tried first and, + * if its runtime is missing (no libvips, FFI disabled, no ext-imagick...), + * the next available driver is used and the failure is logged. A host that + * cannot load the configured driver must degrade to a slower driver, not + * silently stop resizing uploads. * * @param array $options Additional options for ImageManager * @param string|null $driver Driver name to use instead of the configured one */ public static function createImageManager(array $options = [], ?string $driver = null): ImageManager { - $configOptions = config('image.options', []); + $options = array_merge(config('image.options', []), $options); + + if ($driver !== null) { + return self::build($driver, $options); + } + + $configured = (string) config('image.driver', 'vips'); + + if (isset(self::$resolved[$configured])) { + return self::build(self::$resolved[$configured], $options); + } + + $failure = null; - $options = array_merge($configOptions, $options); + foreach (self::candidateDrivers($configured) as $candidate) { + try { + $manager = self::build($candidate, $options); + } catch (Throwable $e) { + $failure ??= $e; + continue; + } + + self::$resolved[$configured] = $candidate; + + if ($candidate !== $configured && $failure) { + Log::error(sprintf( + 'Image driver "%s" is unavailable, falling back to "%s". Uploads are still processed, but fix the driver or set IMAGE_DRIVER=%s. Reason: %s', + $configured, + $candidate, + $candidate, + self::describe($failure) + )); + } + + return $manager; + } + + throw $failure; + } + + /** + * Drivers to try, in order: the configured one, then whatever else this + * PHP build can actually run. + * + * @return array + */ + public static function candidateDrivers(?string $configured = null): array + { + $drivers = [$configured ?? (string) config('image.driver', 'vips')]; + + if (extension_loaded('imagick')) { + $drivers[] = 'imagick'; + } + + if (extension_loaded('gd')) { + $drivers[] = 'gd'; + } + + return array_values(array_unique($drivers)); + } + + /** + * Forget memoized driver resolution (tests, or after fixing a driver + * without restarting the worker). + */ + public static function flush(): void + { + self::$resolved = []; + } + + protected static function build(string $driver, array $options): ImageManager + { return new ImageManager( self::getDriverClass($driver), autoOrientation: (bool) ($options['autoOrientation'] ?? true), @@ -42,4 +129,15 @@ class ImageDriverManager strip: (bool) ($options['strip'] ?? true) ); } + + protected static function describe(Throwable $e): string + { + $messages = []; + + for ($current = $e; $current !== null; $current = $current->getPrevious()) { + $messages[] = class_basename($current).': '.$current->getMessage(); + } + + return implode(' <- ', $messages); + } }