diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php index ebdeb573..6e7c2da2 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/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); + } } diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 37b1d59d..610412fe 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/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, ], ], ], diff --git a/module/Core/src/EventDispatcher/LocateVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php index d02f8be7..4d98f454 100644 --- a/module/Core/src/EventDispatcher/LocateVisit.php +++ b/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], + ); } } } diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php index a01ec996..b56cd97f 100644 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ b/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