From c97d49b98f533159d79f1aaebde9645af6ec98f6 Mon Sep 17 00:00:00 2001 From: drevel Date: Tue, 23 May 2017 22:12:41 -0700 Subject: [PATCH 1/3] catch and log payload handling errors --- src/ApiClient.php | 18 +++++++++++++++--- src/Payload.php | 4 ++-- src/RealTimeClient.php | 15 +++++++++++++-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/ApiClient.php b/src/ApiClient.php index 831219e..d21f00f 100644 --- a/src/ApiClient.php +++ b/src/ApiClient.php @@ -3,6 +3,8 @@ use GuzzleHttp; use Psr\Http\Message\ResponseInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use React\EventLoop\LoopInterface; use React\Promise\Deferred; use Slack\Message\Message; @@ -33,16 +35,26 @@ class ApiClient */ protected $loop; + /** + * @var LoggerInterface + */ + protected $logger; + /** * Creates a new API client instance. * - * @param GuzzleHttp\ClientInterface $httpClient A Guzzle client instance to - * send requests with. + * @param LoopInterface $loop + * @param GuzzleHttp\ClientInterface $httpClient A Guzzle client instance to send requests with. + * @param LoggerInterface $logger */ - public function __construct(LoopInterface $loop, GuzzleHttp\ClientInterface $httpClient = null) + public function __construct( + LoopInterface $loop, + GuzzleHttp\ClientInterface $httpClient = null, + LoggerInterface $logger = null) { $this->loop = $loop; $this->httpClient = $httpClient ?: new GuzzleHttp\Client(); + $this->logger = $logger ?: new NullLogger(); } /** diff --git a/src/Payload.php b/src/Payload.php index e1d657e..c750870 100644 --- a/src/Payload.php +++ b/src/Payload.php @@ -12,11 +12,11 @@ class Payload implements \ArrayAccess, \JsonSerializable protected $data; /** - * Creates a response object from a JSON message. + * Creates a payload object from a JSON message. * * @param string $json A JSON string. * - * @return Response The parsed response. + * @return Payload The parsed message. */ public static function fromJson($json) { diff --git a/src/RealTimeClient.php b/src/RealTimeClient.php index be0480e..dba4b8b 100644 --- a/src/RealTimeClient.php +++ b/src/RealTimeClient.php @@ -6,6 +6,7 @@ use Evenement\EventEmitterTrait; use React\Promise; use Slack\Message\Message; +use Throwable; /** * A client for the Slack real-time messaging API. @@ -361,13 +362,23 @@ public function isConnected() /** * Handles incoming websocket messages, parses them, and emits them as remote events. * - * @param WebSocketMessageInterface $messageRaw A websocket message. + * @param WebSocketMessageInterface $message A websocket message. */ private function onMessage(WebSocketMessageInterface $message) { - // parse the message and get the event name $payload = Payload::fromJson($message->getData()); + try { + $this->handlePayload($payload); + } catch (Throwable $throwable) { + $this->logger->warning('Payload handling error: '.$throwable->getMessage()); + $this->logger->warning('Payload: '.$payload->toJson()); + $this->logger->warning($throwable->getTraceAsString()); + } + } + + private function handlePayload(Payload $payload) + { if (isset($payload['type'])) { switch ($payload['type']) { case 'hello': From afe69b04d024d759eb098beff65a505fdd7f73da Mon Sep 17 00:00:00 2001 From: drevel Date: Wed, 24 May 2017 07:54:20 -0700 Subject: [PATCH 2/3] use Exception for php 5 --- src/RealTimeClient.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/RealTimeClient.php b/src/RealTimeClient.php index dba4b8b..cf5a1cd 100644 --- a/src/RealTimeClient.php +++ b/src/RealTimeClient.php @@ -4,9 +4,9 @@ use Devristo\Phpws\Client\WebSocket; use Devristo\Phpws\Messaging\WebSocketMessageInterface; use Evenement\EventEmitterTrait; +use Exception; use React\Promise; use Slack\Message\Message; -use Throwable; /** * A client for the Slack real-time messaging API. @@ -370,7 +370,7 @@ private function onMessage(WebSocketMessageInterface $message) try { $this->handlePayload($payload); - } catch (Throwable $throwable) { + } catch (Exception $throwable) { $this->logger->warning('Payload handling error: '.$throwable->getMessage()); $this->logger->warning('Payload: '.$payload->toJson()); $this->logger->warning($throwable->getTraceAsString()); From 4d559cdf3014f6095dd89973e9a1906bf8643cab Mon Sep 17 00:00:00 2001 From: drevel Date: Thu, 25 May 2017 11:29:09 -0700 Subject: [PATCH 3/3] log a single error message --- src/RealTimeClient.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/RealTimeClient.php b/src/RealTimeClient.php index cf5a1cd..51bb5d9 100644 --- a/src/RealTimeClient.php +++ b/src/RealTimeClient.php @@ -370,10 +370,12 @@ private function onMessage(WebSocketMessageInterface $message) try { $this->handlePayload($payload); - } catch (Exception $throwable) { - $this->logger->warning('Payload handling error: '.$throwable->getMessage()); - $this->logger->warning('Payload: '.$payload->toJson()); - $this->logger->warning($throwable->getTraceAsString()); + } catch (Exception $exception) { + $context = [ + 'payload' => $message->getData(), + 'stackTrace' => $exception->getTrace(), + ]; + $this->logger->error('Payload handling error: '.$exception->getMessage(), $context); } }