If the underlying storage fails, emit an event

docs/events/events.md

@@ -5,14 +5,16 @@ the flux of other code, in this case into the flux of VichUploaderBundle.
 
 The following is a list of events you can listen to:
 
-| Event name | Event constant | Trigger point|
-|------------|----------------|--------------|
-|`vich_uploader.pre_upload`|`Events::PRE_UPLOAD`|before a file upload is handled|
-|`vich_uploader.post_upload`|`Events::POST_UPLOAD`|right after a file upload is handled|
-|`vich_uploader.pre_inject`|`Events::PRE_INJECT`|before a file is injected into an entity|
-|`vich_uploader.post_inject`|`Events::POST_INJECT`|after a file is injected into an entity|
-|`vich_uploader.pre_remove`|`Events::PRE_REMOVE`|before a file is removed|
-|`vich_uploader.post_remove`|`Events::POST_REMOVE`|after a file is removed|
+| Event name                   | Event constant         | Trigger point                            |
+|------------------------------|------------------------|------------------------------------------|
+| `vich_uploader.pre_upload`   | `Events::PRE_UPLOAD`   | before a file upload is handled          |
+| `vich_uploader.post_upload`  | `Events::POST_UPLOAD`  | right after a file upload is handled     |
+| `vich_uploader.pre_inject`   | `Events::PRE_INJECT`   | before a file is injected into an entity |
+| `vich_uploader.post_inject`  | `Events::POST_INJECT`  | after a file is injected into an entity  |
+| `vich_uploader.pre_remove`   | `Events::PRE_REMOVE`   | before a file is removed                 |
+| `vich_uploader.post_remove`  | `Events::POST_REMOVE`  | after a file is removed                  |
+| `vich_uploader.upload_error` | `Events::UPLOAD_ERROR` | If file upload failed.                   |
+| `vich_uploader.remove_error` | `Events::REMOVE_ERROR` | If file remove failed.                   |
 
 The `vich_uploader.pre_remove` event is cancelable, that means that the actual remove request will not take place,
 and you have to take action.
@@ -51,4 +53,42 @@ services:
             - { name: kernel.event_listener, event: vich_uploader.pre_upload }
 ```
 
+## Error events
+
+The `UPLOAD_ERROR` event happens when writing a file fails, and before an exception is thrown.
+
+If an error occurs while removing a file, then the `REMOVE_ERROR` event gets fired. Failures in removing
+files do not trigger any exceptions unless you choose to throw an error in the event handler for the `REMOVE_ERROR`
+event.
+
+You can use the event to throw an error when the error occurs:
+
+```php
+<?php
+
+namespace App\EventListener;
+
+use Vich\UploaderBundle\Event\ErrorEvent;use Vich\UploaderBundle\Event\Event;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Vich\UploaderBundle\Event\Events;
+
+class RemoveErrorEventListener implements EventSubscriberInterface
+{
+    public static function getSubscribedEvents(){
+            return [
+            Events::REMOVE_ERROR => 'onUploadError',
+        ]       
+    }
+    public function onUploadError(ErrorEvent $errorEvent)
+    {
+        $object = $event->getObject();
+        $mapping = $event->getMapping();
+        $exception = $errorEvent->getThrowable();
+
+        throw $exception;
+    }
+
+}
+```
+
 [Return to the index](../index.md)

src/Event/ErrorEvent.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace Vich\UploaderBundle\Event;
+
+use Vich\UploaderBundle\Mapping\PropertyMapping;
+
+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

@@ -54,4 +54,14 @@ final class Events
      * @Event("Vich\UploaderBundle\Event\Event")
      */
     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

@@ -4,6 +4,7 @@
 
 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 +47,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));
@@ -93,8 +98,11 @@ public function remove(object $obj, string $fieldName): void
         if ($preEvent->isCanceled()) {
             return;
         }
-
-        $this->storage->remove($obj, $mapping);
+        try {
+            $this->storage->remove($obj, $mapping);
+        } 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/FileSystemStorage.php

@@ -41,11 +41,19 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name
     {
         $file = $this->doResolvePath($mapping, $dir, $name);
 
-        return \file_exists($file) && \unlink($file);
+        if (!\file_exists($file) || !unlink($file)) {
+            throw new \Exception('Cannot remove file '.$file);
+        }
+
+        return true;
     }
 
-    protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string
-    {
+    protected function doResolvePath(
+        PropertyMapping $mapping,
+        ?string $dir,
+        string $name,
+        ?bool $relative = false
+    ): string {
         $path = !empty($dir) ? $dir.\DIRECTORY_SEPARATOR.$name : $name;
 
         if ($relative) {

src/Storage/FlysystemStorage.php

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

src/Storage/GaufretteStorage.php

@@ -3,7 +3,6 @@
 namespace Vich\UploaderBundle\Storage;
 
 use Gaufrette\Adapter\MetadataSupporter;
-use Gaufrette\Exception\FileNotFound;
 use Gaufrette\FilesystemInterface;
 use Gaufrette\FilesystemMapInterface;
 use Symfony\Component\HttpFoundation\File\File;
@@ -46,11 +45,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,8 @@ 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,79 @@ 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 testUploadFailsEmitsEventAndException(): void
+    {
+        $this->expectException(\RuntimeException::class);
+
+        $this->expectEvents([Events::PRE_UPLOAD, Events::UPLOAD_ERROR]);
+
+        $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::never())
+            ->method('injectFile')
+            ->with($this->object, $this->mapping);
+
+        $this->handler->upload($this->object, self::FILE_FIELD);
+    }
+
+    public function testremoveFailsWithCannotWriteException(): void
+    {
+        $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 CannotWriteFileException('this is a test'));
+
+        $this->handler->remove($this->object, self::FILE_FIELD);
+    }
+
     public function testRemoveIfEventIsCanceled(): void
     {
         $this->expectEvents([Events::PRE_REMOVE]);

tests/Storage/FileSystemStorageTest.php

@@ -42,6 +42,8 @@ public function testRemoveSkipsEmptyFilenameProperties(?string $propertyValue):
      */
     public function testRemoveSkipsNonExistingFile(): void
     {
+        $this->expectException(\Exception::class);
+
         $this->mapping
             ->expects(self::once())
             ->method('getUploadDir')

tests/Storage/GaufretteStorageTest.php

@@ -172,6 +172,8 @@ public function testThatRemoveMethodDoesDeleteFile(): void
      */
     public function testRemoveNotFoundFile(): void
     {
+        // the exception is caught in the UploadHandler.
+        $this->expectException(FileNotFound::class);
         $this->mapping
             ->method('getUploadDestination')
             ->willReturn('filesystemKey');