If the underlying storage fails, emit an event

If the underlying storage fails, we emit an event that can be handled as
the user wants.

The behaviour is to throw exceptions on write but not on remove as this
seems to be the semantics as they are in the library prior to this
change.

src/Event/ErrorEvent.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace Vich\UploaderBundle\Event;
+
+
+use Vich\UploaderBundle\Mapping\PropertyMapping;
+
+/**
+ * User: tarjei
+ * Date: 10.05.2024 / 07:49
+ */
+class ErrorEvent extends Event
+{
+    private \Throwable $throwable;
+
+    public function __construct(object $object, PropertyMapping $mapping, \Throwable $throwable)
+    {
+        parent::__construct($object, $mapping);
+        $this->throwable = $throwable;
+    }
+
+    public function getThrowable(): \Throwable
+    {
+        return $this->throwable;
+    }
+
+
+}

src/Event/Events.php

@@ -53,5 +53,13 @@ final class Events
      *
      * @Event("Vich\UploaderBundle\Event\Event")
      */
-    public const POST_REMOVE = 'vich_uploader.post_remove';
+    public const POST_REMOVE  = 'vich_uploader.post_remove';
+    /**
+     * Triggered if writing to storage fails
+     */
+    public const UPLOAD_ERROR = 'vich_uploader.upload_error';
+    /**
+     * Triggered if removing the file from storage fails.
+     */
+    public const REMOVE_ERROR = 'vich_uploader.remove_error';
 }

src/Handler/UploadHandler.php

@@ -2,8 +2,10 @@
 
 namespace Vich\UploaderBundle\Handler;
 
+use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
+use Vich\UploaderBundle\Event\ErrorEvent;
 use Vich\UploaderBundle\Event\Event;
 use Vich\UploaderBundle\Event\Events;
 use Vich\UploaderBundle\FileAbstraction\ReplacingFile;
@@ -46,8 +48,12 @@ public function upload(object $obj, string $fieldName): void
         }
 
         $this->dispatch(Events::PRE_UPLOAD, new Event($obj, $mapping));
-
-        $this->storage->upload($obj, $mapping);
+        try {
+            $this->storage->upload($obj, $mapping);
+        } catch(\Throwable $exception) {
+            $this->dispatch(Events::UPLOAD_ERROR, new ErrorEvent($obj, $mapping, $exception));
+            throw $exception;
+        }
         $this->injector->injectFile($obj, $mapping);
 
         $this->dispatch(Events::POST_UPLOAD, new Event($obj, $mapping));
@@ -94,7 +100,13 @@ public function remove(object $obj, string $fieldName): void
             return;
         }
 
-        $this->storage->remove($obj, $mapping);
+        try {
+            $this->storage->remove($obj, $mapping);
+        } catch (CannotWriteFileException $cannotWriteFileException) {
+            throw $cannotWriteFileException;
+        } catch (\Throwable $exception) {
+            $this->dispatch(Events::REMOVE_ERROR, new ErrorEvent($obj, $mapping, $exception));
+        }
         $mapping->erase($obj);
 
         $this->dispatch(Events::POST_REMOVE, new Event($obj, $mapping));

src/Storage/FlysystemStorage.php

@@ -56,13 +56,8 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name
         $fs = $this->getFilesystem($mapping);
         $path = !empty($dir) ? $dir.'/'.$name : $name;
 
-        try {
-            $fs->delete($path);
-
-            return true;
-        } catch (FilesystemException) {
-            return false;
-        }
+        $fs->delete($path);
+        return true;
     }
 
     protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string

src/Storage/GaufretteStorage.php

@@ -46,11 +46,7 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name
         $filesystem = $this->getFilesystem($mapping);
         $path = !empty($dir) ? $dir.'/'.$name : $name;
 
-        try {
-            return $filesystem->delete($path);
-        } catch (FileNotFound) {
-            return false;
-        }
+        return $filesystem->delete($path);
     }
 
     protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string

src/Storage/StorageInterface.php

@@ -23,6 +23,7 @@ public function upload(object $obj, PropertyMapping $mapping): void;
      *
      * @param object          $obj     The object
      * @param PropertyMapping $mapping The mapping representing the field to remove
+     * @throw \Exception      Throws an exception
      */
     public function remove(object $obj, PropertyMapping $mapping): ?bool;
 

tests/Handler/UploadHandlerTest.php

@@ -4,6 +4,7 @@
 
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException;
 use Vich\TestBundle\Entity\Article;
 use Vich\UploaderBundle\Event\Event;
 use Vich\UploaderBundle\Event\Events;
@@ -191,6 +192,84 @@ public function testRemove(): void
         $this->handler->remove($this->object, self::FILE_FIELD);
     }
 
+    public function testRemoveFailsInStorageDriverEmitsEvent(): void
+    {
+        $this->expectEvents([Events::PRE_REMOVE, Events::REMOVE_ERROR, Events::POST_REMOVE]);
+
+        $this->mapping
+            ->expects(self::once())
+            ->method('getFileName')
+            ->with($this->object)
+            ->willReturn('something not null');
+
+        $this->mapping
+            ->expects(self::once())
+            ->method('erase')
+            ->with($this->object);
+
+        $this->storage
+            ->expects(self::once())
+            ->method('remove')
+            ->with($this->object, $this->mapping)
+            ->willThrowException(new \RuntimeException("Test exception"))
+        ;
+
+        $this->handler->remove($this->object, self::FILE_FIELD);
+    }
+
+    public function testUploadFailsEmitsEvent(): void
+    {
+        $this->expectEvents([Events::PRE_UPLOAD, Events::UPLOAD_ERROR, Events::POST_UPLOAD]);
+
+
+        $this->mapping
+            ->expects(self::once())
+            ->method('getFile')
+            ->with($this->object)
+            ->willReturn($this->getUploadedFileMock());
+
+        $this->storage
+            ->expects(self::once())
+            ->method('upload')
+            ->with($this->object, $this->mapping)
+            ->willThrowException(new \RuntimeException("This is a test"));
+
+        $this->injector
+            ->expects(self::once())
+            ->method('injectFile')
+            ->with($this->object, $this->mapping);
+
+        $this->handler->upload($this->object, self::FILE_FIELD);
+
+    }
+
+    public function testremoveFailsWithCannotWriteException(): void
+    {
+        $this->expectException(CannotWriteFileException::class);
+        $this->expectEvents([Events::PRE_REMOVE]);
+
+
+        $this->mapping
+            ->expects(self::once())
+            ->method('getFileName')
+            ->with($this->object)
+            ->willReturn('something not null');
+
+        $this->mapping
+            ->expects(self::never())
+            ->method('erase')
+            ->with($this->object);
+
+        $this->storage
+            ->expects(self::once())
+            ->method('remove')
+            ->with($this->object, $this->mapping)
+            ->willThrowException(new CannotWriteFileException("this is a test"));
+
+
+        $this->handler->remove($this->object, self::FILE_FIELD);
+
+    }
     public function testRemoveIfEventIsCanceled(): void
     {
         $this->expectEvents([Events::PRE_REMOVE]);