Implemented error handling properly in all Event classes

walkor · Nov 4, 2023 · b4a253a · b4a253a
1 parent ba38b9d
commit b4a253a
Show file tree

Hide file tree

Showing 9 changed files with 229 additions and 238 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -25,6 +25,11 @@ parameters:
  - '#Constant STREAM_POLLNONE not found.#'
  - '#Constant STREAM_POLLOUT not found.#'
  - '#Property Workerman\\Events\\Swow::.* has unknown class Swow\\Coroutine as its type.#'
+ -
+ path: src/Events/Event.php
+ reportUnmatched: false
+ messages:
+ - '#Call to an undefined method EventBase::+.#'
  - path: src/Timer.php
  message: '#Call to static method getSuspension\(\) on an unknown class Revolt\\EventLoop.#'
  - path: tests/Pest.php

diff --git a/src/Events/Ev.php b/src/Events/Ev.php
@@ -15,10 +15,6 @@
 
 namespace Workerman\Events;
 
-use EvIo;
-use EvSignal;
-use EvTimer;
-
 /**
  * Ev eventloop
  */
@@ -27,28 +23,28 @@ class Ev implements EventInterface
  /**
  * All listeners for read event.
  *
- * @var array
+ * @var array<int, \EvIo>
  */
  protected array $readEvents = [];
 
  /**
  * All listeners for write event.
  *
- * @var array
+ * @var array<int, \EvIo>
  */
  protected array $writeEvents = [];
 
  /**
  * Event listeners of signal.
  *
- * @var array
+ * @var array<int, \EvSignal>
  */
  protected array $eventSignal = [];
 
  /**
  * All timer event listeners.
  *
- * @var array
+ * @var array<int, \EvTimer>
  */
  protected array $eventTimer = [];
 
@@ -70,9 +66,9 @@ class Ev implements EventInterface
  public function delay(float $delay, callable $func, array $args = []): int
  {
  $timerId = self::$timerId;
- $event = new EvTimer($delay, 0, function () use ($func, $args, $timerId) {
+ $event = new \EvTimer($delay, 0, function () use ($func, $args, $timerId) {
  unset($this->eventTimer[$timerId]);
- $func(...$args);
+ $this->safeCall($func, $args);
  });
  $this->eventTimer[self::$timerId] = $event;
  return self::$timerId++;
@@ -104,9 +100,7 @@ public function offRepeat(int $timerId): bool
  */
  public function repeat(float $interval, callable $func, array $args = []): int
  {
- $event = new EvTimer($interval, $interval, function () use ($func, $args) {
- $func(...$args);
- });
+ $event = new \EvTimer($interval, $interval, fn () => $this->safeCall($func, $args));
  $this->eventTimer[self::$timerId] = $event;
  return self::$timerId++;
  }
@@ -117,9 +111,7 @@ public function repeat(float $interval, callable $func, array $args = []): int
  public function onReadable($stream, callable $func): void
  {
  $fdKey = (int)$stream;
- $event = new EvIo($stream, \Ev::READ, function () use ($func, $stream) {
- $func($stream);
- });
+ $event = new \EvIo($stream, \Ev::READ, fn () => $this->safeCall($func, [$stream]));
  $this->readEvents[$fdKey] = $event;
  }
 
@@ -143,10 +135,8 @@ public function offReadable($stream): bool
  public function onWritable($stream, callable $func): void
  {
  $fdKey = (int)$stream;
- $event = new EvIo($stream, \Ev::WRITE, function () use ($func, $stream) {
- $func($stream);
- });
- $this->readEvents[$fdKey] = $event;
+ $event = new \EvIo($stream, \Ev::WRITE, fn () => $this->safeCall($func, [$stream]));
+ $this->writeEvents[$fdKey] = $event;
  }
 
  /**
@@ -168,9 +158,7 @@ public function offWritable($stream): bool
  */
  public function onSignal(int $signal, callable $func): void
  {
- $event = new EvSignal($signal, function () use ($func, $signal) {
- $func($signal);
- });
+ $event = new \EvSignal($signal, fn () => $this->safeCall($func, [$signal]));
  $this->eventSignal[$signal] = $event;
  }
 
@@ -237,4 +225,22 @@ public function getErrorHandler(): ?callable
  {
  return $this->errorHandler;
  }
+
+ /**
+ * @param callable $func
+ * @param array $args
+ * @return void
+ */
+ private function safeCall(callable $func, array $args = []): void
+ {
+ try {
+ $func(...$args);
+ } catch (\Throwable $e) {
+ if ($this->errorHandler === null) {
+ echo $e;
+ } else {
+ ($this->errorHandler)($e);
+ }
+ }
+ }
 }
diff --git a/src/Events/Event.php b/src/Events/Event.php
@@ -16,56 +16,56 @@
 
 namespace Workerman\Events;
 
-use EventBase;
-use RuntimeException;
-use Throwable;
-use function class_exists;
-use function count;
-
 /**
  * libevent eventloop
  */
 class Event implements EventInterface
 {
  /**
  * Event base.
- * @var EventBase
+ *
+ * @var \EventBase
  */
- protected EventBase $eventBase;
+ protected \EventBase $eventBase;
 
  /**
  * All listeners for read event.
- * @var array
+ *
+ * @var array<int, \Event>
  */
  protected array $readEvents = [];
 
  /**
  * All listeners for write event.
- * @var array
+ *
+ * @var array<int, \Event>
  */
  protected array $writeEvents = [];
 
  /**
  * Event listeners of signal.
- * @var array
+ *
+ * @var array<int, \Event>
  */
  protected array $eventSignal = [];
 
  /**
  * All timer event listeners.
- * [func, args, event, flag, time_interval]
- * @var array
+ *
+ * @var array<int, \Event>
  */
  protected array $eventTimer = [];
 
  /**
  * Timer id.
+ *
  * @var int
  */
  protected int $timerId = 0;
 
  /**
  * Event class name.
+ *
  * @var string
  */
  protected string $eventClassName = '';
@@ -77,17 +77,16 @@ class Event implements EventInterface
 
  /**
  * Construct.
- * @return void
  */
  public function __construct()
  {
- if (class_exists('\\\\Event', false)) {
+ if (\class_exists('\\\\Event', false)) {
  $className = '\\\\Event';
  } else {
  $className = '\Event';
  }
  $this->eventClassName = $className;
- if (class_exists('\\\\EventBase', false)) {
+ if (\class_exists('\\\\EventBase', false)) {
  $className = '\\\\EventBase';
  } else {
  $className = '\EventBase';
@@ -104,14 +103,10 @@ public function delay(float $delay, callable $func, array $args = []): int
  $timerId = $this->timerId++;
  $event = new $className($this->eventBase, -1, $className::TIMEOUT, function () use ($func, $args, $timerId) {
  unset($this->eventTimer[$timerId]);
- try {
- $func(...$args);
- } catch (Throwable $e) {
- $this->error($e);
- }
+ $this->safeCall($func, $args);
  });
  if (!$event->addTimer($delay)) {
- throw new RuntimeException("Event::addTimer($delay) failed");
+ throw new \RuntimeException("Event::addTimer($delay) failed");
  }
  $this->eventTimer[$timerId] = $event;
  return $timerId;
@@ -145,15 +140,9 @@ public function repeat(float $interval, callable $func, array $args = []): int
  {
  $className = $this->eventClassName;
  $timerId = $this->timerId++;
- $event = new $className($this->eventBase, -1, $className::TIMEOUT | $className::PERSIST, function () use ($func, $args) {
- try {
- $func(...$args);
- } catch (Throwable $e) {
- $this->error($e);
- }
- });
+ $event = new $className($this->eventBase, -1, $className::TIMEOUT | $className::PERSIST, fn () => $this->safeCall($func, $args));
  if (!$event->addTimer($interval)) {
- throw new RuntimeException("Event::addTimer($interval) failed");
+ throw new \RuntimeException("Event::addTimer($interval) failed");
  }
  $this->eventTimer[$timerId] = $event;
  return $timerId;
@@ -166,12 +155,10 @@ public function onReadable($stream, callable $func): void
  {
  $className = $this->eventClassName;
  $fdKey = (int)$stream;
- $event = new $this->eventClassName($this->eventBase, $stream, $className::READ | $className::PERSIST, $func, $stream);
- // @phpstan-ignore-next-line Negated boolean expression is always false.
- if (!$event || !$event->add()) {
- return;
+ $event = new $className($this->eventBase, $stream, $className::READ | $className::PERSIST, fn () => $this->safeCall($func, [$stream]));
+ if ($event->add()) {
+ $this->readEvents[$fdKey] = $event;
  }
- $this->readEvents[$fdKey] = $event;
  }
 
  /**
@@ -195,12 +182,10 @@ public function onWritable($stream, callable $func): void
  {
  $className = $this->eventClassName;
  $fdKey = (int)$stream;
- $event = new $this->eventClassName($this->eventBase, $stream, $className::WRITE | $className::PERSIST, $func, $stream);
- // @phpstan-ignore-next-line Negated boolean expression is always false.
- if (!$event || !$event->add()) {
- return;
+ $event = new $className($this->eventBase, $stream, $className::WRITE | $className::PERSIST, fn () => $this->safeCall($func, [$stream]));
+ if ($event->add()) {
+ $this->writeEvents[$fdKey] = $event;
  }
- $this->writeEvents[$fdKey] = $event;
  }
 
  /**
@@ -224,11 +209,10 @@ public function onSignal(int $signal, callable $func): void
  {
  $className = $this->eventClassName;
  $fdKey = $signal;
- $event = $className::signal($this->eventBase, $signal, $func);
- if (!$event || !$event->add()) {
- return;
+ $event = $className::signal($this->eventBase, $signal, fn () => $this->safeCall($func, [$signal]));
+ if ($event->add()) {
+ $this->eventSignal[$fdKey] = $event;
  }
- $this->eventSignal[$fdKey] = $event;
  }
 
  /**
@@ -277,7 +261,7 @@ public function stop(): void
  */
  public function getTimerCount(): int
  {
- return count($this->eventTimer);
+ return \count($this->eventTimer);
  }
 
  /**
@@ -297,20 +281,20 @@ public function getErrorHandler(): ?callable
  }
 
  /**
- * @param Throwable $e
+ * @param callable $func
+ * @param array $args
  * @return void
- * @throws Throwable
  */
- public function error(Throwable $e): void
+ private function safeCall(callable $func, array $args = []): void
  {
  try {
- if (!$this->errorHandler) {
- throw new $e;
+ $func(...$args);
+ } catch (\Throwable $e) {
+ if ($this->errorHandler === null) {
+ echo $e;
+ } else {
+ ($this->errorHandler)($e);
  }
- ($this->errorHandler)($e);
- } catch (Throwable $e) {
- // Cannot trigger an exception in the Event callback, otherwise it will cause an infinite loop
- echo $e;
  }
  }
 }