Use ClosedException as mandated by the ByteStream interfaces

This commit also changes end() to wait for the close() to finish before returning.

lib/BlockingHandle.php

@@ -2,9 +2,11 @@
 
 namespace Amp\File;
 
+use Amp\ByteStream\ClosedException;
 use Amp\Failure;
 use Amp\Promise;
 use Amp\Success;
+use function Amp\call;
 
 class BlockingHandle implements Handle {
     private $fh;
@@ -33,7 +35,7 @@ class BlockingHandle implements Handle {
      */
     public function read(int $length = self::DEFAULT_READ_LENGTH): Promise {
         if ($this->fh === null) {
-            throw new FilesystemException("The file has been closed");
+            throw new ClosedException("The file has been closed");
         }
 
         $data = \fread($this->fh, $length);
@@ -52,7 +54,7 @@ class BlockingHandle implements Handle {
      */
     public function write(string $data): Promise {
         if ($this->fh === null) {
-            throw new FilesystemException("The file has been closed");
+            throw new ClosedException("The file has been closed");
         }
 
         $len = \fwrite($this->fh, $data);
@@ -70,9 +72,14 @@ class BlockingHandle implements Handle {
      * {@inheritdoc}
      */
     public function end(string $data = ""): Promise {
+        return call(function () use ($data) {
             $promise = $this->write($data);
-        $promise->onResolve([$this, "close"]);
+
+            // ignore any errors
+            yield Promise\any([$this->close()]);
+
             return $promise;
+        });
     }
 
     /**

lib/EioHandle.php

@@ -2,6 +2,7 @@
 
 namespace Amp\File;
 
+use Amp\ByteStream\ClosedException;
 use Amp\Deferred;
 use Amp\Promise;
 use Amp\Success;
@@ -61,7 +62,7 @@ class EioHandle implements Handle {
         $this->isActive = false;
 
         if ($result === -1) {
-            $deferred->fail(new FilesystemException(
+            $deferred->fail(new ClosedException(
                 sprintf('Reading from file failed: %s.', \eio_get_last_error($req))
             ));
         } else {
@@ -79,7 +80,7 @@ class EioHandle implements Handle {
         }
 
         if (!$this->writable) {
-            throw new \Error("The file is no longer writable");
+            throw new ClosedException("The file is no longer writable");
         }
 
         $this->isActive = true;
@@ -122,15 +123,20 @@ class EioHandle implements Handle {
      * {@inheritdoc}
      */
     public function end(string $data = ""): Promise {
+        return call(function () use ($data) {
             $promise = $this->write($data);
             $this->writable = false;
-        $promise->onResolve([$this, "close"]);
+
+            // ignore any errors
+            yield Promise\any([$this->close()]);
+
             return $promise;
+        });
     }
 
     private function onWrite(Deferred $deferred, $result, $req) {
         if ($this->queue->isEmpty()) {
-            $deferred->fail(new FilesystemException('No pending write, the file may have been closed'));
+            $deferred->fail(new ClosedException('No pending write, the file may have been closed'));
         }
 
         $this->queue->shift();
@@ -139,7 +145,7 @@ class EioHandle implements Handle {
         }
 
         if ($result === -1) {
-            $deferred->fail(new FilesystemException(
+            $deferred->fail(new ClosedException(
                 sprintf('Writing to the file failed: %s', \eio_get_last_error($req))
             ));
         } else {

lib/FilesystemException.php

@@ -2,7 +2,9 @@
 
 namespace Amp\File;
 
-class FilesystemException extends \Exception {
+use Amp\ByteStream\StreamException;
+
+class FilesystemException extends StreamException {
     public function __construct(string $message, \Throwable $previous = null) {
         parent::__construct($message, 0, $previous);
     }

lib/ParallelHandle.php

@@ -2,12 +2,14 @@
 
 namespace Amp\File;
 
+use Amp\ByteStream\ClosedException;
 use Amp\Coroutine;
 use Amp\Parallel\Worker\TaskException;
 use Amp\Parallel\Worker\Worker;
 use Amp\Parallel\Worker\WorkerException;
 use Amp\Promise;
 use Amp\Success;
+use function Amp\call;
 
 class ParallelHandle implements Handle {
     /** @var \Amp\Parallel\Worker\Worker */
@@ -90,7 +92,7 @@ class ParallelHandle implements Handle {
 
     public function read(int $length = self::DEFAULT_READ_LENGTH): Promise {
         if ($this->id === null) {
-            throw new FilesystemException("The file has been closed");
+            throw new ClosedException("The file has been closed");
         }
 
         if ($this->busy) {
@@ -121,7 +123,7 @@ class ParallelHandle implements Handle {
      */
     public function write(string $data): Promise {
         if ($this->id === null) {
-            throw new FilesystemException("The file has been closed");
+            throw new ClosedException("The file has been closed");
         }
 
         if ($this->busy && $this->pendingWrites === 0) {
@@ -129,7 +131,7 @@ class ParallelHandle implements Handle {
         }
 
         if (!$this->writable) {
-            throw new \Error("The file is no longer writable");
+            throw new ClosedException("The file is no longer writable");
         }
 
         return new Coroutine($this->doWrite($data));
@@ -139,10 +141,15 @@ class ParallelHandle implements Handle {
      * {@inheritdoc}
      */
     public function end(string $data = ""): Promise {
+        return call(function () use ($data) {
             $promise = $this->write($data);
             $this->writable = false;
-        $promise->onResolve([$this, "close"]);
+
+            // ignore any errors
+            yield Promise\any([$this->close()]);
+
             return $promise;
+        });
     }
 
     private function doWrite(string $data): \Generator {
@@ -170,7 +177,7 @@ class ParallelHandle implements Handle {
      */
     public function seek(int $offset, int $whence = SEEK_SET): Promise {
         if ($this->id === null) {
-            throw new FilesystemException("The file has been closed");
+            throw new ClosedException("The file has been closed");
         }
 
         if ($this->busy) {

lib/UvHandle.php

@@ -2,6 +2,7 @@
 
 namespace Amp\File;
 
+use Amp\ByteStream\ClosedException;
 use Amp\Deferred;
 use Amp\File\Internal\UvPoll;
 use Amp\Loop;
@@ -57,7 +58,7 @@ class UvHandle implements Handle {
             $this->isActive = false;
 
             if ($result < 0) {
-                $deferred->fail(new FilesystemException(\uv_strerror($result)));
+                $deferred->fail(new ClosedException(\uv_strerror($result)));
             } else {
                 $length = strlen($buffer);
                 $this->position = $this->position + $length;
@@ -79,7 +80,7 @@ class UvHandle implements Handle {
         }
 
         if (!$this->writable) {
-            throw new \Error("The file is no longer writable");
+            throw new ClosedException("The file is no longer writable");
         }
 
         $this->isActive = true;
@@ -103,10 +104,15 @@ class UvHandle implements Handle {
      * {@inheritdoc}
      */
     public function end(string $data = ""): Promise {
+        return call(function () use ($data) {
             $promise = $this->write($data);
             $this->writable = false;
-        $promise->onResolve([$this, "close"]);
+
+            // ignore any errors
+            yield Promise\any([$this->close()]);
+
             return $promise;
+        });
     }
 
     private function push(string $data): Promise {
@@ -117,7 +123,7 @@ class UvHandle implements Handle {
 
         $onWrite = function ($fh, $result) use ($deferred, $length) {
             if ($this->queue->isEmpty()) {
-                $deferred->fail(new FilesystemException('No pending write, the file may have been closed'));
+                $deferred->fail(new ClosedException('No pending write, the file may have been closed'));
             }
 
             $this->queue->shift();
@@ -126,9 +132,7 @@ class UvHandle implements Handle {
             }
 
             if ($result < 0) {
-                $deferred->fail(new FilesystemException(
+                $deferred->fail(new ClosedException(\uv_strerror($result)));
-                    \uv_strerror($result)
-                ));
             } else {
                 StatCache::clear($this->path);
                 $newPosition = $this->position + $length;

test/HandleTest.php

@@ -2,6 +2,7 @@
 
 namespace Amp\File\Test;
 
+use Amp\ByteStream\ClosedException;
 use Amp\File;
 use Amp\PHPUnit\TestCase;
 
@@ -36,6 +37,33 @@ abstract class HandleTest extends TestCase {
         });
     }
 
+    public function testWriteAfterClose() {
+        $this->execute(function () {
+            $path = Fixture::path() . "/write";
+            /** @var \Amp\File\Handle $handle */
+            $handle = yield File\open($path, "c+");
+            $this->assertSame(0, $handle->tell());
+            yield $handle->write("foo");
+            yield $handle->close();
+
+            $this->expectException(ClosedException::class);
+            yield $handle->write("bar");
+        });
+    }
+
+    public function testWriteAfterEnd() {
+        $this->execute(function () {
+            $path = Fixture::path() . "/write";
+            /** @var \Amp\File\Handle $handle */
+            $handle = yield File\open($path, "c+");
+            $this->assertSame(0, $handle->tell());
+            yield $handle->end("foo");
+
+            $this->expectException(ClosedException::class);
+            yield $handle->write("bar");
+        });
+    }
+
     public function testReadingToEof() {
         $this->execute(function () {
             /** @var \Amp\File\Handle $handle */
@@ -150,14 +178,13 @@ abstract class HandleTest extends TestCase {
         });
     }
 
-    /**
-     * @expectedException \Amp\File\FilesystemException
-     */
     public function testClose() {
         $this->execute(function () {
             /** @var \Amp\File\Handle $handle */
             $handle = yield File\open(__FILE__, "r");
             yield $handle->close();
+
+            $this->expectException(ClosedException::class);
             yield $handle->read();
         });
     }