Extended error handling on LocateVisit handler

module/CLI/src/Util/GeolocationDbUpdater.php

@@ -60,24 +60,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
         }
     }
 
-    /**
-     * @throws GeolocationDbUpdateFailedException
-     */
-    private function downloadNewDb(bool $olderDbExists, ?callable $beforeDownload, ?callable $handleProgress): void
-    {
-        if ($beforeDownload !== null) {
-            $beforeDownload($olderDbExists);
-        }
-
-        try {
-            $this->dbUpdater->downloadFreshCopy($handleProgress);
-        } catch (RuntimeException $e) {
-            throw $olderDbExists
-                ? GeolocationDbUpdateFailedException::withOlderDb($e)
-                : GeolocationDbUpdateFailedException::withoutOlderDb($e);
-        }
-    }
-
     private function buildIsTooOld(Metadata $meta): bool
     {
         $buildTimestamp = $this->resolveBuildTimestamp($meta);
@@ -105,4 +87,31 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
 
         throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch);
     }
+
+    /**
+     * @throws GeolocationDbUpdateFailedException
+     */
+    private function downloadNewDb(bool $olderDbExists, ?callable $beforeDownload, ?callable $handleProgress): void
+    {
+        if ($beforeDownload !== null) {
+            $beforeDownload($olderDbExists);
+        }
+
+        try {
+            $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists));
+        } catch (RuntimeException $e) {
+            throw $olderDbExists
+                ? GeolocationDbUpdateFailedException::withOlderDb($e)
+                : GeolocationDbUpdateFailedException::withoutOlderDb($e);
+        }
+    }
+
+    private function wrapHandleProgressCallback(?callable $handleProgress, bool $olderDbExists): ?callable
+    {
+        if ($handleProgress === null) {
+            return null;
+        }
+
+        return fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists);
+    }
 }

module/Core/config/event_dispatcher.config.php

@@ -22,7 +22,7 @@ return [
             EventDispatcher\Event\VisitLocated::class => [
                 EventDispatcher\NotifyVisitToMercure::class,
                 EventDispatcher\NotifyVisitToWebHooks::class,
-//                EventDispatcher\UpdateGeoliteDb::class,
+//                EventDispatcher\UpdateGeoLiteDb::class,
             ],
         ],
     ],

module/Core/src/EventDispatcher/LocateVisit.php

@@ -15,6 +15,7 @@ use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
+use Throwable;
 
 class LocateVisit
 {
@@ -62,27 +63,6 @@ class LocateVisit
         $this->eventDispatcher->dispatch(new VisitLocated($visitId));
     }
 
-//    private function downloadOrUpdateGeoLiteDb(string $visitId): bool
-//    {
-//        try {
-//            $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists): void {
-//                $this->logger->notice(sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading'));
-//            });
-//        } catch (GeolocationDbUpdateFailedException $e) {
-//            if (! $e->olderDbExists()) {
-//                $this->logger->error(
-//                    'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}',
-//                    ['e' => $e, 'visitId' => $visitId],
-//                );
-//                return false;
-//            }
-//
-//            $this->logger->warning('GeoLite2 database update failed. Proceeding with old version. {e}', ['e' => $e]);
-//        }
-//
-//        return true;
-//    }
-
     private function locateVisit(string $visitId, ?string $originalIpAddress, Visit $visit): void
     {
         $isLocatable = $originalIpAddress !== null || $visit->isLocatable();
@@ -98,6 +78,11 @@ class LocateVisit
                 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}',
                 ['e' => $e, 'visitId' => $visitId],
             );
+        } catch (Throwable $e) {
+            $this->logger->error(
+                'An unexpected error occurred while trying to locate visit with id "{visitId}". {e}',
+                ['e' => $e, 'visitId' => $visitId],
+            );
         }
     }
 }

module/Core/test/EventDispatcher/LocateVisitTest.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core\EventDispatcher;
 
 use Doctrine\ORM\EntityManagerInterface;
+use OutOfRangeException;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
@@ -110,7 +111,7 @@ class LocateVisitTest extends TestCase
             WrongIpException::class,
         );
         $logWarning = $this->logger->warning(
-            Argument::containingString('Tried to locate visit with id "{visitId}", but its address seems to be wrong.'),
+            'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}',
             Argument::type('array'),
         );
         $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function (): void {
@@ -125,6 +126,32 @@ class LocateVisitTest extends TestCase
         $dispatch->shouldHaveBeenCalledOnce();
     }
 
+    /** @test */
+    public function unhandledExceptionLogsError(): void
+    {
+        $event = new UrlVisited('123');
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn(
+            Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
+        );
+        $resolveLocation = $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->willThrow(
+            OutOfRangeException::class,
+        );
+        $logError = $this->logger->error(
+            'An unexpected error occurred while trying to locate visit with id "{visitId}". {e}',
+            Argument::type('array'),
+        );
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function (): void {
+        });
+
+        ($this->locateVisit)($event);
+
+        $findVisit->shouldHaveBeenCalledOnce();
+        $resolveLocation->shouldHaveBeenCalledOnce();
+        $logError->shouldHaveBeenCalled();
+        $this->em->flush()->shouldNotHaveBeenCalled();
+        $dispatch->shouldHaveBeenCalledOnce();
+    }
+
     /**
      * @test
      * @dataProvider provideNonLocatableVisits