Merge pull request #141 from jolicode/fix-error-handling

Logout user on auth or user retrieval exception

config/services.yaml

@@ -13,6 +13,12 @@ services:
         # this means you cannot fetch services directly from the container via $container->get()
         # if you need to do this, you can override this setting on individual services
         public: false
+        bind:
+            iterable $applications: !tagged_iterator secret_santa.application
+
+    _instanceof:
+        JoliCode\SecretSanta\Application\ApplicationInterface:
+            tags: ['secret_santa.application']
 
     # makes classes in src/ available to be used as services
     # this creates a service per class whose id is the fully-qualified class name
@@ -32,10 +38,6 @@ services:
     # add more services, or override services that need manual wiring
     JoliCode\SecretSanta\Controller\SantaController:
         public: true
-        arguments:
-            $applications:
-                - '@JoliCode\SecretSanta\Application\SlackApplication'
-                - '@JoliCode\SecretSanta\Application\DiscordApplication'
 
     JoliCode\SecretSanta\Controller\SlackController:
         public: true

src/Application/DiscordApplication.php

@@ -23,6 +23,8 @@
 
 class DiscordApplication implements ApplicationInterface
 {
+    const APPLICATION_CODE = 'discord';
+
     const SESSION_KEY_STATE = 'santa.discord.state';
 
     private const SESSION_KEY_TOKEN = 'santa.discord.token';
@@ -47,7 +49,7 @@ public function __construct(RequestStack $requestStack, ApiHelper $apiHelper, Us
 
     public function getCode(): string
     {
-        return 'discord';
+        return self::APPLICATION_CODE;
     }
 
     public function isAuthenticated(): bool

src/Application/SlackApplication.php

@@ -21,6 +21,8 @@
 
 class SlackApplication implements ApplicationInterface
 {
+    const APPLICATION_CODE = 'slack';
+
     const SESSION_KEY_STATE = 'santa.slack.state';
 
     private const SESSION_KEY_TOKEN = 'santa.slack.token';
@@ -39,7 +41,7 @@ public function __construct(RequestStack $requestStack, UserExtractor $userExtra
 
     public function getCode(): string
     {
-        return 'slack';
+        return self::APPLICATION_CODE;
     }
 
     public function isAuthenticated(): bool

src/Controller/DiscordController.php

@@ -70,11 +70,11 @@ public function authenticate(Request $request, DiscordApplication $discordApplic
         } elseif (empty($request->query->get('state')) || ($request->query->get('state') !== $session->get(DiscordApplication::SESSION_KEY_STATE))) {
             $session->remove(DiscordApplication::SESSION_KEY_STATE);
 
-            throw new AuthenticationException('Invalid OAuth state.');
+            throw new AuthenticationException(DiscordApplication::APPLICATION_CODE, 'Invalid OAuth state.');
         }
 
         if (!$request->query->has('guild_id')) {
-            throw new AuthenticationException('No guild_id found.');
+            throw new AuthenticationException(DiscordApplication::APPLICATION_CODE, 'No guild_id found.');
         }
 
         try {
@@ -87,7 +87,7 @@ public function authenticate(Request $request, DiscordApplication $discordApplic
             /** @var DiscordResourceOwner $user */
             $user = $provider->getResourceOwner($token);
         } catch (\Exception $e) {
-            throw new AuthenticationException('Failed to retrieve data from Discord.', 0, $e);
+            throw new AuthenticationException(DiscordApplication::APPLICATION_CODE, 'Failed to retrieve data from Discord.', $e);
         }
 
         $discordApplication->setToken($token);

src/Controller/SantaController.php

@@ -40,7 +40,7 @@ class SantaController extends AbstractController
     private $statisticCollector;
     private $bugsnag;
 
-    public function __construct(RouterInterface $router, Environment $twig, LoggerInterface $logger, array $applications,
+    public function __construct(RouterInterface $router, Environment $twig, LoggerInterface $logger, iterable $applications,
                                 StatisticCollector $statistic, Client $bugsnag)
     {
         $this->router = $router;

src/Controller/SlackController.php

@@ -70,7 +70,7 @@ public function authenticate(Request $request, SlackApplication $slackApplicatio
         } elseif (empty($request->query->get('state')) || ($request->query->get('state') !== $session->get(SlackApplication::SESSION_KEY_STATE))) {
             $session->remove(SlackApplication::SESSION_KEY_STATE);
 
-            throw new AuthenticationException('Invalid OAuth state.');
+            throw new AuthenticationException(SlackApplication::APPLICATION_CODE, 'Invalid OAuth state.');
         }
 
         try {
@@ -83,7 +83,7 @@ public function authenticate(Request $request, SlackApplication $slackApplicatio
             /** @var SlackResourceOwner $user */
             $user = $provider->getResourceOwner($token);
         } catch (\Exception $e) {
-            throw new AuthenticationException('Failed to retrieve data from Slack.', 0, $e);
+            throw new AuthenticationException(SlackApplication::APPLICATION_CODE, 'Failed to retrieve data from Slack.', $e);
         }
 
         $slackApplication->setToken($token);

src/Discord/MessageSender.php

@@ -63,7 +63,11 @@ public function sendSecretMessage(SecretSanta $secretSanta, string $giver, strin
             $precision = null;
 
             if (($response = $e->getResponse()) && 403 === $response->getStatusCode()) {
-                $precision = 'The user does not allow to receive DM on this server. Please ask them to change their server settings.';
+                $precision = sprintf(
+                    '@%s does not allow to receive DM on the server "%s". Please ask them to change their server privacy settings as explained in our faq.',
+                    $secretSanta->getUser($giver)->getName(),
+                    $secretSanta->getOrganization()
+                );
             }
 
             throw new MessageSendFailedException($secretSanta, $secretSanta->getUser($giver), $e, $precision);

src/Discord/UserExtractor.php

@@ -11,6 +11,7 @@
 
 namespace JoliCode\SecretSanta\Discord;
 
+use JoliCode\SecretSanta\Application\DiscordApplication;
 use JoliCode\SecretSanta\Exception\UserExtractionFailedException;
 use JoliCode\SecretSanta\Model\Group;
 use JoliCode\SecretSanta\Model\User;
@@ -40,7 +41,7 @@ public function extractForGuild(int $guildId): array
         $startTime = time();
         do {
             if ((time() - $startTime) > 19) {
-                throw new UserExtractionFailedException('Took too much time to retrieve all the users on your team.');
+                throw new UserExtractionFailedException(DiscordApplication::APPLICATION_CODE, 'Took too much time to retrieve all the users on your team.');
             }
 
             $lastMember = $lastMembers ? end($lastMembers) : null;
@@ -49,7 +50,7 @@ public function extractForGuild(int $guildId): array
                 /** @var GuildMember[] $members */
                 $lastMembers = $this->apiHelper->getMembersInGuild($guildId, $lastMember ? $lastMember->user->id : null);
             } catch (\Throwable $t) {
-                throw new UserExtractionFailedException('Could not fetch members in guild.', 0, $t);
+                throw new UserExtractionFailedException(DiscordApplication::APPLICATION_CODE, 'Could not fetch members in guild.', $t);
             }
 
             $members = array_merge($members, $lastMembers);

src/EventListener/HandleExceptionSubscriber.php

@@ -12,6 +12,7 @@
 namespace JoliCode\SecretSanta\EventListener;
 
 use Bugsnag\Client;
+use JoliCode\SecretSanta\Application\ApplicationInterface;
 use JoliCode\SecretSanta\Exception\AuthenticationException;
 use JoliCode\SecretSanta\Exception\UserExtractionFailedException;
 use Psr\Log\LoggerInterface;
@@ -26,18 +27,21 @@ class HandleExceptionSubscriber implements EventSubscriberInterface
     private $logger;
     private $twig;
     private $bugsnag;
+    private $applications;
 
-    public function __construct(LoggerInterface $logger, Environment $twig, Client $bugsnag)
+    public function __construct(LoggerInterface $logger, Environment $twig, Client $bugsnag, iterable $applications)
     {
         $this->logger = $logger;
         $this->twig = $twig;
         $this->bugsnag = $bugsnag;
+        $this->applications = $applications;
     }
 
     public function handleException(ExceptionEvent $event)
     {
         $exception = $event->getThrowable();
         $statusCode = null;
+        $applicationCode = null;
 
         if ($exception instanceof AuthenticationException) {
             $this->logger->error(sprintf('Authentication error: %s', $exception->getMessage()), [
@@ -49,6 +53,8 @@ public function handleException(ExceptionEvent $event)
             });
 
             $statusCode = 401;
+
+            $applicationCode = $exception->getApplicationCode();
         } elseif ($exception instanceof UserExtractionFailedException) {
             $this->logger->error('Could not retrieve users', [
                 'exception' => $exception,
@@ -58,13 +64,23 @@ public function handleException(ExceptionEvent $event)
                 $report->setSeverity('error');
             });
 
+            $applicationCode = $exception->getApplicationCode();
+
             $statusCode = 500;
         }
 
         if (!$statusCode) {
             return;
         }
 
+        if ($applicationCode) {
+            $application = $this->getApplication($applicationCode);
+
+            if ($application) {
+                $application->reset();
+            }
+        }
+
         $response = new Response($this->twig->render('error.html.twig', [
             'exception' => $exception,
         ]), $statusCode);
@@ -79,4 +95,15 @@ public static function getSubscribedEvents()
             KernelEvents::EXCEPTION => ['handleException', 255],
         ];
     }
+
+    private function getApplication(string $code): ?ApplicationInterface
+    {
+        foreach ($this->applications as $application) {
+            if ($application->getCode() === $code) {
+                return $application;
+            }
+        }
+
+        return null;
+    }
 }

src/Exception/ApplicationRelatedException.php

@@ -0,0 +1,17 @@
+<?php
+
+/*
+ * This file is part of the Secret Santa project.
+ *
+ * (c) JoliCode <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace JoliCode\SecretSanta\Exception;
+
+interface ApplicationRelatedException
+{
+    public function getApplicationCode(): string;
+}

src/Exception/AuthenticationException.php

@@ -11,6 +11,19 @@
 
 namespace JoliCode\SecretSanta\Exception;
 
-class AuthenticationException extends \RuntimeException
+class AuthenticationException extends \RuntimeException implements ApplicationRelatedException
 {
+    private $applicationCode;
+
+    public function __construct(string $applicationCode, $message = '', \Throwable $previous = null)
+    {
+        $this->applicationCode = $applicationCode;
+
+        parent::__construct($message, 0, $previous);
+    }
+
+    public function getApplicationCode(): string
+    {
+        return $this->applicationCode;
+    }
 }

src/Exception/MessageSendFailedException.php

@@ -24,7 +24,7 @@ public function __construct(SecretSanta $secretSanta, User $recipient, \Throwabl
         $this->secretSanta = $secretSanta;
         $this->recipient = $recipient;
 
-        parent::__construct(sprintf('Fail to send message to %s.%s', $recipient->getName(), $precision ? ' ' . $precision : ''), 0, $previous);
+        parent::__construct(sprintf('Fail to send message to @%s.%s', $recipient->getName(), $precision ? ' ' . $precision : ''), 0, $previous);
     }
 
     public function getSecretSanta(): SecretSanta

src/Exception/UserExtractionFailedException.php

@@ -11,6 +11,19 @@
 
 namespace JoliCode\SecretSanta\Exception;
 
-class UserExtractionFailedException extends \RuntimeException
+class UserExtractionFailedException extends \RuntimeException implements ApplicationRelatedException
 {
+    private $applicationCode;
+
+    public function __construct(string $applicationCode, $message = '', \Throwable $previous = null)
+    {
+        $this->applicationCode = $applicationCode;
+
+        parent::__construct($message, 0, $previous);
+    }
+
+    public function getApplicationCode(): string
+    {
+        return $this->applicationCode;
+    }
 }

src/Slack/UserExtractor.php

@@ -11,6 +11,7 @@
 
 namespace JoliCode\SecretSanta\Slack;
 
+use JoliCode\SecretSanta\Application\SlackApplication;
 use JoliCode\SecretSanta\Exception\UserExtractionFailedException;
 use JoliCode\SecretSanta\Model\Group;
 use JoliCode\SecretSanta\Model\User;
@@ -37,7 +38,7 @@ public function extractAll(string $token): array
         $startTime = time();
         do {
             if ((time() - $startTime) > 19) {
-                throw new UserExtractionFailedException('Took too much time to retrieve all the users on your team.');
+                throw new UserExtractionFailedException(SlackApplication::APPLICATION_CODE, 'Took too much time to retrieve all the users on your team.');
             }
 
             try {
@@ -47,10 +48,10 @@ public function extractAll(string $token): array
                 ]);
 
                 if (!$response->getOk()) {
-                    throw new UserExtractionFailedException('Could not fetch members in team.');
+                    throw new UserExtractionFailedException(SlackApplication::APPLICATION_CODE, 'Could not fetch members in team.');
                 }
             } catch (\Throwable $t) {
-                throw new UserExtractionFailedException('Could not fetch members in team.', 0, $t);
+                throw new UserExtractionFailedException(SlackApplication::APPLICATION_CODE, 'Could not fetch members in team.', $t);
             }
 
             $slackUsers = array_merge($slackUsers, $response->getMembers());

templates/content/faq.html.twig

@@ -97,7 +97,7 @@
                             <img src="{{ asset('images/discord-privacy-settings.gif') }}" alt="How to change the privacy settings on Discord" />
                         </div>
                         <p>
-                            Once they made the change, you can safely retry to send the messages. When the draw is finished,
+                            Once they made the change, you can safely retry to send the remaining messages. When the draw is finished,
                             they can of course re-disable this setting as they wish.
                         </p>
                     </li>

templates/santa/finish.html.twig

@@ -58,24 +58,24 @@
                         If a user <strong>does not allow to receive private message on a server</strong>, Discord will prevent us to send
                         them a message. You need to ask them to <strong>change their privacy settings</strong> on this server.
                         See <a href="{{ url('faq', {_fragment: 'discord-server-dm'}) }}" target="_blank">our FAQ</a> for more details.
-                        Once they made the change, you can safely retry to send the messages.
+                        Once they made the change, you can safely retry to send the remaining messages. See below.
                     </p>
                 {% endif %}
 
                 <p>
                     Note that errors can also be due to <strong>temporary network problems</strong> when sending messages.
-                    So simply <strong>retrying to send messages</strong> could fix the problem. See below.
+                    So simply <strong>retrying to send the remaining messages</strong> could fix the problem. See below.
                 </p>
 
-                <h3>Retry to send the message</h3>
+                <h3>Retry to send the remaining message</h3>
 
                 <p>
                     Please <strong>click the Continue</strong> button below to <strong>safely</strong> inform the remaining users as if nothing happened.
                 </p>
 
                 <div class="is-center">
                     <a href="{{ path('send_messages', {hash: secretSanta.hash}) }}" class="big-button warning-btn" id="retry-button">
-                        <span class="fas fa-exclamation-triangle" aria-hidden="true"></span>
+                        <span class="fas fa-redo" aria-hidden="true"></span>
                         Continue - Send the remaining messages
                     </a>
                 </div>
@@ -107,7 +107,7 @@
 
         <div class="is-center">
             <a href="{{ path('homepage') }}" class="big-button">
-                <span class="fas fa-redo" aria-hidden="true"></span>
+                <span class="fas fa-plus" aria-hidden="true"></span>
                 Start another Secret Santa
             </a>
         </div>