PHPLIB-592: Allow GridFS StreamWrapper to throw exceptions (#869)

Previously, exceptions were converted to E_USER_WARNING with a false return value for consistency with PHP's stream API (e.g. fread). This is not actually required and allowing the original exception to be propagated is likely more helpful for users and simplifies some handling in the spec test runner.

Author: jmikola

src/GridFS/StreamWrapper.php

@@ -19,20 +19,15 @@
 
 use MongoDB\BSON\UTCDateTime;
 use stdClass;
-use Throwable;
 
 use function explode;
-use function get_class;
 use function in_array;
 use function is_integer;
-use function sprintf;
 use function stream_context_get_options;
 use function stream_get_wrappers;
 use function stream_wrapper_register;
 use function stream_wrapper_unregister;
-use function trigger_error;
 
-use const E_USER_WARNING;
 use const SEEK_CUR;
 use const SEEK_END;
 use const SEEK_SET;
@@ -162,13 +157,7 @@ public function stream_read($length)
             return '';
         }
 
-        try {
-            return $this->stream->readBytes($length);
-        } catch (Throwable $e) {
-            trigger_error(sprintf('%s: %s', get_class($e), $e->getMessage()), E_USER_WARNING);
-
-            return false;
-        }
+        return $this->stream->readBytes($length);
     }
 
     /**
@@ -259,13 +248,7 @@ public function stream_write($data)
             return 0;
         }
 
-        try {
-            return $this->stream->writeBytes($data);
-        } catch (Throwable $e) {
-            trigger_error(sprintf('%s: %s', get_class($e), $e->getMessage()), E_USER_WARNING);
-
-            return false;
-        }
+        return $this->stream->writeBytes($data);
     }
 
     /**

tests/GridFS/BucketFunctionalTest.php

@@ -9,6 +9,7 @@
 use MongoDB\Driver\WriteConcern;
 use MongoDB\Exception\InvalidArgumentException;
 use MongoDB\GridFS\Bucket;
+use MongoDB\GridFS\Exception\CorruptFileException;
 use MongoDB\GridFS\Exception\FileNotFoundException;
 use MongoDB\GridFS\Exception\StreamException;
 use MongoDB\Model\BSONDocument;
@@ -173,7 +174,8 @@ public function testDownloadingFileWithMissingChunk(): void
 
         $this->chunksCollection->deleteOne(['files_id' => $id, 'n' => 0]);
 
-        $this->expectWarning();
+        $this->expectException(CorruptFileException::class);
+        $this->expectExceptionMessage('Chunk not found for index "0"');
         stream_get_contents($this->bucket->openDownloadStream($id));
     }
 
@@ -186,7 +188,8 @@ public function testDownloadingFileWithUnexpectedChunkIndex(): void
             ['$set' => ['n' => 1]]
         );
 
-        $this->expectWarning();
+        $this->expectException(CorruptFileException::class);
+        $this->expectExceptionMessage('Expected chunk to have index "0" but found "1"');
         stream_get_contents($this->bucket->openDownloadStream($id));
     }
 
@@ -199,7 +202,8 @@ public function testDownloadingFileWithUnexpectedChunkSize(): void
             ['$set' => ['data' => new Binary('fooba', Binary::TYPE_GENERIC)]]
         );
 
-        $this->expectWarning();
+        $this->expectException(CorruptFileException::class);
+        $this->expectExceptionMessage('Expected chunk to have size "6" but found "5"');
         stream_get_contents($this->bucket->openDownloadStream($id));
     }
 

tests/UnifiedSpecTests/Operation.php

@@ -17,8 +17,8 @@
 use MongoDB\Operation\FindOneAndReplace;
 use MongoDB\Operation\FindOneAndUpdate;
 use PHPUnit\Framework\Assert;
-use PHPUnit\Framework\AssertionFailedError;
 use PHPUnit\Framework\Constraint\IsType;
+use PHPUnit\Framework\Exception as PHPUnitException;
 use stdClass;
 use Throwable;
 
@@ -146,18 +146,14 @@ public function assert(bool $rethrowExceptions = false): void
             $result = $this->execute();
             $saveResultAsEntity = $this->saveResultAsEntity;
         } catch (Throwable $e) {
-            /* Rethrow any internal PHP errors and PHPUnit assertion failures,
-             * since those are never expected for "expectError".
-             *
-             * Note: we must be selective about what PHPUnit exceptions to pass
-             * through, as PHPUnit's Warning exception must be considered for
-             * expectError in GridFS tests (see: PHPLIB-592).
+            /* Rethrow any internal PHP errors and PHPUnit exceptions, since
+             * those are never expected for "expectError".
              *
              * TODO: Consider adding operation details (e.g. operations[] index)
              * to the exception message. Alternatively, throw a new exception
              * and include this as the previous, since PHPUnit will render the
              * chain when reporting a test failure. */
-            if ($e instanceof Error || $e instanceof AssertionFailedError) {
+            if ($e instanceof Error || $e instanceof PHPUnitException) {
                 throw $e;
             }