From 5797059008eb5562eb59ba7d317f63b573503170 Mon Sep 17 00:00:00 2001 From: Jonas Kattendick Date: Fri, 13 Feb 2026 23:26:32 +0100 Subject: [PATCH] fix!: handle php errors with custom error handling This commit changes the api of the stream classes, since indication of success or failure is no longer necessary. --- src/Child.php | 10 +++--- src/ChildStderr.php | 2 +- src/ChildStdin.php | 2 +- src/ChildStdout.php | 2 +- src/Command.php | 66 ++++++++++++++++++++++++++++------------ src/CommandException.php | 11 ++++++- src/Stdio.php | 14 ++------- src/StdioInterface.php | 2 +- src/StreamException.php | 10 ++++++ src/StreamReadTrait.php | 52 +++++++++++++++++++++++++++++++ src/StreamReadable.php | 48 ----------------------------- src/StreamTrait.php | 52 +++++++++++++++++++++++++++++++ src/StreamWritable.php | 46 ---------------------------- src/StreamWriteTrait.php | 50 ++++++++++++++++++++++++++++++ 14 files changed, 232 insertions(+), 135 deletions(-) create mode 100644 src/StreamException.php create mode 100644 src/StreamReadTrait.php delete mode 100644 src/StreamReadable.php create mode 100644 src/StreamTrait.php delete mode 100644 src/StreamWritable.php create mode 100644 src/StreamWriteTrait.php diff --git a/src/Child.php b/src/Child.php index 74a5aba..a871141 100644 --- a/src/Child.php +++ b/src/Child.php @@ -50,6 +50,10 @@ final class Child */ public function __construct(mixed $process, array $pipes) { + if (get_resource_type($process) !== 'process') { + throw new ChildException('invalid resource type: not a process'); + } + $this->process = $process; $status = proc_get_status($this->process); @@ -94,10 +98,6 @@ final class Child return $this->status; } - if (!is_resource($this->process)) { - throw new ChildException('Resource was already closed'); - } - // Avoid possible deadlock before waiting. $this->stdin?->close(); @@ -165,7 +165,7 @@ final class Child public function __destruct() { - if (is_resource($this->process)) { + if (get_resource_type($this->process) === 'process') { proc_close($this->process); } } diff --git a/src/ChildStderr.php b/src/ChildStderr.php index 2318846..95e4814 100644 --- a/src/ChildStderr.php +++ b/src/ChildStderr.php @@ -8,7 +8,7 @@ use Override; final class ChildStderr implements StdioInterface { - use StreamReadable; + use StreamReadTrait; #[Override] public function getDescriptionSpec(int $fd): mixed diff --git a/src/ChildStdin.php b/src/ChildStdin.php index c2dc2bd..870db17 100644 --- a/src/ChildStdin.php +++ b/src/ChildStdin.php @@ -8,7 +8,7 @@ use Override; final class ChildStdin implements StdioInterface { - use StreamWritable; + use StreamWriteTrait; #[Override] public function getDescriptionSpec(int $fd): mixed diff --git a/src/ChildStdout.php b/src/ChildStdout.php index 6394def..054675b 100644 --- a/src/ChildStdout.php +++ b/src/ChildStdout.php @@ -8,7 +8,7 @@ use Override; final class ChildStdout implements StdioInterface { - use StreamReadable; + use StreamReadTrait; #[Override] public function getDescriptionSpec(int $fd): mixed diff --git a/src/Command.php b/src/Command.php index 4886610..390164e 100644 --- a/src/Command.php +++ b/src/Command.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Nih\CommandBuilder; +use InvalidArgumentException; use Override; use Stringable; use ValueError; @@ -202,10 +203,19 @@ final class Command implements Stringable * * Defaults to inherit when used with spawn or status, and defaults to piped * when used with output. + * + * @param StdioInterface|resource $stdin + * + * @throws InvalidArgumentException + * When the provided stdin handle is neither a StdioInterface nor a live + * stream resource. */ - public function stdin(StdioInterface $stdin): static + public function stdin(mixed $stdin): static { - $this->stdin = $stdin; + $this->stdin = $stdin instanceof StdioInterface + ? $stdin + : Stdio::stream($stdin); + return $this; } @@ -214,10 +224,19 @@ final class Command implements Stringable * * Defaults to inherit when used with spawn or status, and defaults to piped * when used with output. + * + * @param StdioInterface|resource $stdout + * + * @throws InvalidArgumentException + * When the provided stdout handle is neither a StdioInterface nor a live + * stream resource. */ - public function stdout(StdioInterface $stdout): static + public function stdout(mixed $stdout): static { - $this->stdout = $stdout; + $this->stdout = $stdout instanceof StdioInterface + ? $stdout + : Stdio::stream($stdout); + return $this; } @@ -226,10 +245,19 @@ final class Command implements Stringable * * Defaults to inherit when used with spawn or status, and defaults to piped * when used with output. + * + * @param StdioInterface|resource $stderr + * + * @throws InvalidArgumentException + * When the provided stderr handle is neither a StdioInterface nor a live + * stream resource. */ - public function stderr(StdioInterface $stderr): static + public function stderr(mixed $stderr): static { - $this->stderr = $stderr; + $this->stderr = $stderr instanceof StdioInterface + ? $stderr + : Stdio::stream($stderr); + return $this; } @@ -385,21 +413,21 @@ final class Command implements Stringable } } - $proc = proc_open( - [$program, ...$this->args], - $descriptorSpec, - $pipes, - $this->cwd, - $environment, - ); - - if ($proc === false) { - throw new CommandException(sprintf( - 'Program "%s" failed to start', - $this->program, - )); + try { + set_error_handler(CommandException::handleError(...)); + $proc = proc_open( + [$program, ...$this->args], + $descriptorSpec, + $pipes, + $this->cwd, + $environment, + ); + } finally { + restore_error_handler(); } + assert($proc !== false); + return new Child($proc, $pipes); } } diff --git a/src/CommandException.php b/src/CommandException.php index 8c109b0..26ebeb6 100644 --- a/src/CommandException.php +++ b/src/CommandException.php @@ -7,4 +7,13 @@ namespace Nih\CommandBuilder; use RuntimeException; class CommandException extends RuntimeException -{} +{ + public static function handleError(int $errno, string $errstr, string $file, int $line): never + { + $exception = new self($errstr); + $exception->file = $file; + $exception->line = $line; + + throw $exception; + } +} diff --git a/src/Stdio.php b/src/Stdio.php index e8eecb0..b4de55b 100644 --- a/src/Stdio.php +++ b/src/Stdio.php @@ -100,22 +100,12 @@ abstract class Stdio * * @param resource $stream * - * @throws InvalidArgumentException When stream is not a live stream resource + * @throws InvalidArgumentException When stream is not a stream resource */ public static function stream($stream): StdioInterface { - $isResource = is_resource($stream) - || ($stream !== null - && !is_scalar($stream) - && !is_array($stream) - && !is_object($stream)); - - if (!$isResource) { - throw new InvalidArgumentException('not a resource'); - } - if (get_resource_type($stream) !== 'stream') { - throw new InvalidArgumentException('resource is not a stream or was closed'); + throw new InvalidArgumentException('resource is not a stream'); } return new class($stream) implements StdioInterface { diff --git a/src/StdioInterface.php b/src/StdioInterface.php index fa847c7..2b8fa98 100644 --- a/src/StdioInterface.php +++ b/src/StdioInterface.php @@ -14,7 +14,7 @@ use Stringable; interface StdioInterface { /** - * @return resource|array{0: 'file', 1: string, 2: string}|array{0: 'piped', 1: string} + * @return resource|array{0: 'file', 1: string, 2: string}|array{0: 'pipe', 1: string} */ public function getDescriptionSpec(int $fd): mixed; } diff --git a/src/StreamException.php b/src/StreamException.php new file mode 100644 index 0000000..be4cd8a --- /dev/null +++ b/src/StreamException.php @@ -0,0 +1,10 @@ +stream) !== 'stream') { + throw new StreamException('the stream was closed'); + } + + try { + set_error_handler(StreamException::handleError(...)); + $bytes = fread($this->stream, $length); + } finally { + restore_error_handler(); + } + + assert($bytes !== false); + + return $bytes; + } + + /** + * @throws StreamException + */ + public function getContents(?int $length = null, int $offset = -1): string + { + if (get_resource_type($this->stream) !== 'stream') { + throw new StreamException('the stream was closed'); + } + + try { + set_error_handler(StreamException::handleError(...)); + $contents = stream_get_contents($this->stream, $length, $offset); + } finally { + restore_error_handler(); + } + + assert($contents !== false); + + return $contents; + } +} diff --git a/src/StreamReadable.php b/src/StreamReadable.php deleted file mode 100644 index 4e02b48..0000000 --- a/src/StreamReadable.php +++ /dev/null @@ -1,48 +0,0 @@ -stream)) { - throw new CommandException('Cannot read from closed stream'); - } - - return fread($this->stream, $length) ?: null; - } - - public function getContents(?int $length = null, int $offset = -1): ?string - { - if (!is_resource($this->stream)) { - throw new CommandException('Cannot read from closed stream'); - } - - $contents = stream_get_contents($this->stream, $length, $offset); - return $contents === false - ? null - : $contents; - } - - public function close(): bool - { - return is_resource($this->stream) - ? fclose($this->stream) - : true; - } - - public function __destruct() - { - $this->close(); - } -} diff --git a/src/StreamTrait.php b/src/StreamTrait.php new file mode 100644 index 0000000..eb3b822 --- /dev/null +++ b/src/StreamTrait.php @@ -0,0 +1,52 @@ +stream) !== 'stream') { + return; + } + + try { + set_error_handler(StreamException::handleError(...)); + $success = fclose($this->stream); + } finally { + restore_error_handler(); + } + + assert($success); + } + + public function __destruct() + { + if (get_resource_type($this->stream) === 'stream') { + fclose($this->stream); + } + } +} diff --git a/src/StreamWritable.php b/src/StreamWritable.php deleted file mode 100644 index 34e9a83..0000000 --- a/src/StreamWritable.php +++ /dev/null @@ -1,46 +0,0 @@ -stream)) { - throw new CommandException('Cannot write to closed stream'); - } - - $bytes = fwrite($this->stream, $data, $length) ?: null; - return $bytes === false ? null : $bytes; - } - - public function flush(): bool - { - if (!is_resource($this->stream)) { - throw new CommandException('Cannot flush closed stream'); - } - - return fflush($this->stream); - } - - public function close(): bool - { - return is_resource($this->stream) - ? fclose($this->stream) - : true; - } - - public function __destruct() - { - $this->close(); - } -} diff --git a/src/StreamWriteTrait.php b/src/StreamWriteTrait.php new file mode 100644 index 0000000..fdbdfd1 --- /dev/null +++ b/src/StreamWriteTrait.php @@ -0,0 +1,50 @@ +stream) !== 'stream') { + throw new StreamException('the stream was closed'); + } + + try { + set_error_handler(StreamException::handleError(...)); + $bytes = fwrite($this->stream, $data, $length); + } finally { + restore_error_handler(); + } + + assert($bytes !== false); + + return $bytes; + } + + /** + * @throws StreamException + */ + public function flush(): void + { + if (get_resource_type($this->stream) !== 'stream') { + throw new StreamException('the stream was closed'); + } + + try { + set_error_handler(StreamException::handleError(...)); + $success = fflush($this->stream); + } finally { + restore_error_handler(); + } + + assert($success); + } +}