Updated CLI commands to just print exception messages when possible

module/CLI/src/Command/Api/DisableKeyCommand.php

@@ -4,8 +4,8 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI\Command\Api;
 
-use InvalidArgumentException;
 use Shlinkio\Shlink\CLI\Util\ExitCodes;
+use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
@@ -45,7 +45,7 @@ class DisableKeyCommand extends Command
             $io->success(sprintf('API key "%s" properly disabled', $apiKey));
             return ExitCodes::EXIT_SUCCESS;
         } catch (InvalidArgumentException $e) {
-            $io->error(sprintf('API key "%s" does not exist.', $apiKey));
+            $io->error($e->getMessage());
             return ExitCodes::EXIT_FAILURE;
         }
     }

module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php

@@ -56,21 +56,16 @@ class DeleteShortUrlCommand extends Command
             $this->runDelete($io, $shortCode, $ignoreThreshold);
             return ExitCodes::EXIT_SUCCESS;
         } catch (Exception\ShortUrlNotFoundException $e) {
-            $io->error(sprintf('Provided short code "%s" could not be found.', $shortCode));
+            $io->error($e->getMessage());
             return ExitCodes::EXIT_FAILURE;
         } catch (Exception\DeleteShortUrlException $e) {
-            return $this->retry($io, $shortCode, $e);
+            return $this->retry($io, $shortCode, $e->getMessage());
         }
     }
 
-    private function retry(SymfonyStyle $io, string $shortCode, Exception\DeleteShortUrlException $e): int
+    private function retry(SymfonyStyle $io, string $shortCode, string $warningMsg): int
     {
-        $warningMsg = sprintf(
+        $io->writeln(sprintf('<bg=yellow>%s</>', $warningMsg));
-            'It was not possible to delete the short URL with short code "%s" because it has more than %s visits.',
-            $shortCode,
-            $e->getVisitsThreshold()
-        );
-        $io->writeln('<bg=yellow>' . $warningMsg . '</>');
         $forceDelete = $io->confirm('Do you want to delete it anyway?', false);
 
         if ($forceDelete) {

module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php

@@ -141,13 +141,8 @@ class GenerateShortUrlCommand extends Command
                 sprintf('Generated short URL: <info>%s</info>', $shortUrl->toString($this->domainConfig)),
             ]);
             return ExitCodes::EXIT_SUCCESS;
-        } catch (InvalidUrlException $e) {
+        } catch (InvalidUrlException | NonUniqueSlugException $e) {
-            $io->error(sprintf('Provided URL "%s" is invalid. Try with a different one.', $longUrl));
+            $io->error($e->getMessage());
-            return ExitCodes::EXIT_FAILURE;
-        } catch (NonUniqueSlugException $e) {
-            $io->error(
-                sprintf('Provided slug "%s" is already in use by another URL. Try with a different one.', $customSlug)
-            );
             return ExitCodes::EXIT_FAILURE;
         }
     }

module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php

@@ -65,7 +65,7 @@ class ResolveUrlCommand extends Command
             $output->writeln(sprintf('Long URL: <info>%s</info>', $url->getLongUrl()));
             return ExitCodes::EXIT_SUCCESS;
         } catch (ShortUrlNotFoundException $e) {
-            $io->error(sprintf('Provided short code "%s" could not be found.', $shortCode));
+            $io->error($e->getMessage());
             return ExitCodes::EXIT_FAILURE;
         }
     }

module/CLI/src/Command/Tag/RenameTagCommand.php

@@ -13,8 +13,6 @@ use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 
-use function sprintf;
-
 class RenameTagCommand extends Command
 {
     public const NAME = 'tag:rename';
@@ -48,7 +46,7 @@ class RenameTagCommand extends Command
             $io->success('Tag properly renamed.');
             return ExitCodes::EXIT_SUCCESS;
         } catch (TagNotFoundException $e) {
-            $io->error(sprintf('A tag with name "%s" was not found', $oldName));
+            $io->error($e->getMessage());
             return ExitCodes::EXIT_FAILURE;
         }
     }

module/CLI/src/Exception/GeolocationDbUpdateFailedException.php

@@ -12,20 +12,16 @@ class GeolocationDbUpdateFailedException extends RuntimeException implements Exc
     /** @var bool */
     private $olderDbExists;
 
-    public function __construct(bool $olderDbExists, string $message = '', int $code = 0, ?Throwable $previous = null)
-    {
-        $this->olderDbExists = $olderDbExists;
-        parent::__construct($message, $code, $previous);
-    }
-
     public static function create(bool $olderDbExists, ?Throwable $prev = null): self
     {
-        return new self(
+        $e = new self(
-            $olderDbExists,
             'An error occurred while updating geolocation database, and an older version could not be found',
             0,
             $prev
         );
+        $e->olderDbExists = $olderDbExists;
+
+        return $e;
     }
 
     public function olderDbExists(): bool

module/CLI/src/Util/GeolocationDbUpdater.php

@@ -10,7 +10,6 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Lock\Factory as Locker;
-use Throwable;
 
 class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
 {
@@ -40,8 +39,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
 
         try {
             $this->downloadIfNeeded($mustBeUpdated, $handleProgress);
-        } catch (Throwable $e) {
-            throw $e;
         } finally {
             $lock->release();
         }

module/CLI/test/Command/Api/DisableKeyCommandTest.php

@@ -29,7 +29,7 @@ class DisableKeyCommandTest extends TestCase
     }
 
     /** @test */
-    public function providedApiKeyIsDisabled()
+    public function providedApiKeyIsDisabled(): void
     {
         $apiKey = 'abcd1234';
         $this->apiKeyService->disable($apiKey)->shouldBeCalledOnce();
@@ -43,17 +43,18 @@ class DisableKeyCommandTest extends TestCase
     }
 
     /** @test */
-    public function errorIsReturnedIfServiceThrowsException()
+    public function errorIsReturnedIfServiceThrowsException(): void
     {
         $apiKey = 'abcd1234';
-        $disable = $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class);
+        $expectedMessage = 'API key "abcd1234" does not exist.';
+        $disable = $this->apiKeyService->disable($apiKey)->willThrow(new InvalidArgumentException($expectedMessage));
 
         $this->commandTester->execute([
             'apiKey' => $apiKey,
         ]);
         $output = $this->commandTester->getDisplay();
 
-        $this->assertStringContainsString('API key "abcd1234" does not exist.', $output);
+        $this->assertStringContainsString($expectedMessage, $output);
         $disable->shouldHaveBeenCalledOnce();
     }
 }

module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php

@@ -58,13 +58,13 @@ class DeleteShortUrlCommandTest extends TestCase
     {
         $shortCode = 'abc123';
         $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow(
-            Exception\ShortUrlNotFoundException::class
+            Exception\ShortUrlNotFoundException::fromNotFoundShortCode($shortCode)
         );
 
         $this->commandTester->execute(['shortCode' => $shortCode]);
         $output = $this->commandTester->getDisplay();
 
-        $this->assertStringContainsString(sprintf('Provided short code "%s" could not be found.', $shortCode), $output);
+        $this->assertStringContainsString(sprintf('No URL found with short code "%s"', $shortCode), $output);
         $deleteByShortCode->shouldHaveBeenCalledOnce();
     }
 
@@ -79,11 +79,11 @@ class DeleteShortUrlCommandTest extends TestCase
     ): void {
         $shortCode = 'abc123';
         $deleteByShortCode = $this->service->deleteByShortCode($shortCode, Argument::type('bool'))->will(
-            function (array $args) {
+            function (array $args) use ($shortCode) {
                 $ignoreThreshold = array_pop($args);
 
                 if (!$ignoreThreshold) {
-                    throw Exception\DeleteShortUrlException::fromVisitsThreshold(10, '');
+                    throw Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode);
                 }
             }
         );
@@ -93,7 +93,7 @@ class DeleteShortUrlCommandTest extends TestCase
         $output = $this->commandTester->getDisplay();
 
         $this->assertStringContainsString(sprintf(
-            'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.',
+            'Impossible to delete short URL with short code "%s" since it has more than "10" visits.',
             $shortCode
         ), $output);
         $this->assertStringContainsString($expectedMessage, $output);
@@ -112,7 +112,7 @@ class DeleteShortUrlCommandTest extends TestCase
     {
         $shortCode = 'abc123';
         $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow(
-            Exception\DeleteShortUrlException::fromVisitsThreshold(10, '')
+            Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode)
         );
         $this->commandTester->setInputs(['no']);
 
@@ -120,7 +120,7 @@ class DeleteShortUrlCommandTest extends TestCase
         $output = $this->commandTester->getDisplay();
 
         $this->assertStringContainsString(sprintf(
-            'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.',
+            'Impossible to delete short URL with short code "%s" since it has more than "10" visits.',
             $shortCode
         ), $output);
         $this->assertStringContainsString('Short URL was not deleted.', $output);

module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php

@@ -59,21 +59,22 @@ class GenerateShortUrlCommandTest extends TestCase
     /** @test */
     public function exceptionWhileParsingLongUrlOutputsError(): void
     {
-        $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(new InvalidUrlException())
+        $url = 'http://domain.com/invalid';
+        $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(InvalidUrlException::fromUrl($url))
                                                                ->shouldBeCalledOnce();
 
-        $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid']);
+        $this->commandTester->execute(['longUrl' => $url]);
         $output = $this->commandTester->getDisplay();
 
         $this->assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode());
-        $this->assertStringContainsString('Provided URL "http://domain.com/invalid" is invalid.', $output);
+        $this->assertStringContainsString('Provided URL http://domain.com/invalid is invalid.', $output);
     }
 
     /** @test */
     public function providingNonUniqueSlugOutputsError(): void
     {
         $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(
-            NonUniqueSlugException::class
+            NonUniqueSlugException::fromSlug('my-slug')
         );
 
         $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid', '--customSlug' => 'my-slug']);

module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php

@@ -52,11 +52,12 @@ class ResolveUrlCommandTest extends TestCase
     public function incorrectShortCodeOutputsErrorMessage(): void
     {
         $shortCode = 'abc123';
-        $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(ShortUrlNotFoundException::class)
+        $this->urlShortener->shortCodeToUrl($shortCode, null)
-                                                             ->shouldBeCalledOnce();
+            ->willThrow(ShortUrlNotFoundException::fromNotFoundShortCode($shortCode))
+            ->shouldBeCalledOnce();
 
         $this->commandTester->execute(['shortCode' => $shortCode]);
         $output = $this->commandTester->getDisplay();
-        $this->assertStringContainsString(sprintf('Provided short code "%s" could not be found', $shortCode), $output);
+        $this->assertStringContainsString(sprintf('No URL found with short code "%s"', $shortCode), $output);
     }
 }

module/CLI/test/Command/Tag/RenameTagCommandTest.php

@@ -34,11 +34,11 @@ class RenameTagCommandTest extends TestCase
     }
 
     /** @test */
-    public function errorIsPrintedIfExceptionIsThrown()
+    public function errorIsPrintedIfExceptionIsThrown(): void
     {
         $oldName = 'foo';
         $newName = 'bar';
-        $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(TagNotFoundException::class);
+        $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(TagNotFoundException::fromTag('foo'));
 
         $this->commandTester->execute([
             'oldName' => $oldName,
@@ -46,12 +46,12 @@ class RenameTagCommandTest extends TestCase
         ]);
         $output = $this->commandTester->getDisplay();
 
-        $this->assertStringContainsString('A tag with name "foo" was not found', $output);
+        $this->assertStringContainsString('Tag with name "foo" could not be found', $output);
         $renameTag->shouldHaveBeenCalled();
     }
 
     /** @test */
-    public function successIsPrintedIfNoErrorOccurs()
+    public function successIsPrintedIfNoErrorOccurs(): void
     {
         $oldName = 'foo';
         $newName = 'bar';

module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php

@@ -12,47 +12,6 @@ use Throwable;
 
 class GeolocationDbUpdateFailedExceptionTest extends TestCase
 {
-    /**
-     * @test
-     * @dataProvider provideOlderDbExists
-     */
-    public function constructCreatesExceptionWithDefaultArgs(bool $olderDbExists): void
-    {
-        $e = new GeolocationDbUpdateFailedException($olderDbExists);
-
-        $this->assertEquals($olderDbExists, $e->olderDbExists());
-        $this->assertEquals('', $e->getMessage());
-        $this->assertEquals(0, $e->getCode());
-        $this->assertNull($e->getPrevious());
-    }
-
-    public function provideOlderDbExists(): iterable
-    {
-        yield 'with older DB' => [true];
-        yield 'without older DB' => [false];
-    }
-
-    /**
-     * @test
-     * @dataProvider provideConstructorArgs
-     */
-    public function constructCreatesException(bool $olderDbExists, string $message, int $code, ?Throwable $prev): void
-    {
-        $e = new GeolocationDbUpdateFailedException($olderDbExists, $message, $code, $prev);
-
-        $this->assertEquals($olderDbExists, $e->olderDbExists());
-        $this->assertEquals($message, $e->getMessage());
-        $this->assertEquals($code, $e->getCode());
-        $this->assertEquals($prev, $e->getPrevious());
-    }
-
-    public function provideConstructorArgs(): iterable
-    {
-        yield [true, 'This is a nice error message', 99, new Exception('prev')];
-        yield [false, 'Another message', 0, new RuntimeException('prev')];
-        yield [true, 'An yet another message', -50, null];
-    }
-
     /**
      * @test
      * @dataProvider provideCreateArgs

module/Core/src/Exception/NonUniqueSlugException.php

@@ -17,7 +17,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem
     private const TITLE = 'Invalid custom slug';
     private const TYPE = 'INVALID_SLUG';
 
-    public static function fromSlug(string $slug, ?string $domain): self
+    public static function fromSlug(string $slug, ?string $domain = null): self
     {
         $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain);
         $e = new self(sprintf('Provided slug "%s" is already in use%s.', $slug, $suffix));