Merge pull request #570 from jolicode/exception

Better search for exception origin
jolicode · Nov 14, 2024 · 2d5a474 · 2d5a474
2 parents 265bdd7 + 914d4ff
commit 2d5a474
Show file tree

Hide file tree

Showing 23 changed files with 106 additions and 142 deletions.
diff --git a/src/Console/Application.php b/src/Console/Application.php
@@ -61,6 +61,8 @@ public function getHelp(): string
     public function renderThrowable(\Throwable $e, OutputInterface $output): void
     {
         if (!$output->isVerbose()) {
+            $this->enhanceException($e);
+
             if ($e instanceof ProblemException) {
                 $this->io->error($e->getMessage());
 
@@ -103,6 +105,36 @@ protected function getDefaultInputDefinition(): InputDefinition
         return $definition;
     }
 
+    private function enhanceException(\Throwable $exception): \Throwable
+    {
+        $castorDirs = [
+            \dirname(__DIR__, 1),
+            \dirname(__DIR__, 2) . \DIRECTORY_SEPARATOR . 'vendor',
+            \dirname(__DIR__, 2) . \DIRECTORY_SEPARATOR . 'bin',
+            // Useful for the phar, or static binary
+            $_SERVER['SCRIPT_NAME'],
+        ];
+
+        foreach ($exception->getTrace() as $frame) {
+            if (!\array_key_exists('file', $frame) || !\array_key_exists('line', $frame)) {
+                continue;
+            }
+
+            foreach ($castorDirs as $dir) {
+                if (str_starts_with($frame['file'], $dir)) {
+                    continue 2;
+                }
+            }
+
+            (new \ReflectionProperty(\Exception::class, 'file'))->setValue($exception, $frame['file']);
+            (new \ReflectionProperty(\Exception::class, 'line'))->setValue($exception, $frame['line']);
+
+            break;
+        }
+
+        return $exception;
+    }
+
     private function getLogo(): string
     {
         if (!($_SERVER['CASTOR_TEST'] ?? false)) {

diff --git a/src/Import/Importer.php b/src/Import/Importer.php
@@ -10,7 +10,6 @@
 use Symfony\Component\Finder\Finder;
 
 use function Castor\Internal\castor_require;
-use function Castor\Internal\fix_exception;
 
 /** @internal */
 class Importer
@@ -47,9 +46,9 @@ public function import(string $path, ?string $file = null): void
 
                 return;
             } catch (ImportError $e) {
-                throw $this->createImportException($package, $e->getMessage(), $e);
+                throw $this->createImportException($package, $e->getMessage());
             } catch (RemoteNotAllowed $e) {
-                $this->logger->warning($this->getImportLocatedMessage($path, $e->getMessage(), 1));
+                $this->logger->warning(\sprintf('Could not import "%s": %s', $path, $e->getMessage()));
 
                 return;
             }
@@ -97,27 +96,8 @@ public function getImports(): array
         return array_keys($this->imports);
     }
 
-    private function getImportLocatedMessage(string $path, string $reason, int $depth): string
+    private function createImportException(string $path, string $message): \InvalidArgumentException
     {
-        /** @var array{file: string, line: int} $caller */
-        $caller = debug_backtrace()[$depth + 1];
-
-        return \sprintf(
-            'Could not import "%s" in "%s" on line %d. Reason: %s',
-            $path,
-            $caller['file'],
-            $caller['line'],
-            $reason,
-        );
-    }
-
-    private function createImportException(string $path, string $message, ?\Throwable $e = null): \Throwable
-    {
-        $depth = 2;
-
-        return fix_exception(
-            new \InvalidArgumentException($this->getImportLocatedMessage($path, $message, $depth), previous: $e),
-            $depth
-        );
+        return new \InvalidArgumentException(\sprintf('Could not import "%s": %s', $path, $message));
     }
 }
diff --git a/src/functions-internal.php b/src/functions-internal.php
@@ -17,22 +17,3 @@ function castor_require(string $file): void
 
     require_once $file;
 }
-
-/**
- * Remove the last internal frames (like the call to run()) to display a nice message to the end user.
- *
- * @internal
- */
-function fix_exception(\Throwable $exception, int $depth = 0): \Throwable
-{
-    $lastFrame = $exception->getTrace()[$depth];
-    foreach (['file', 'line'] as $key) {
-        if (!\array_key_exists($key, $lastFrame)) {
-            continue;
-        }
-        $r = new \ReflectionProperty(\Exception::class, $key);
-        $r->setValue($exception, $lastFrame[$key]);
-    }
-
-    return $exception;
-}
diff --git a/src/functions.php b/src/functions.php
@@ -32,7 +32,6 @@
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 
-use function Castor\Internal\fix_exception;
 use function Symfony\Component\String\u;
 
 /**
@@ -71,26 +70,22 @@ function run(
         $workingDirectory = $path;
     }
 
-    try {
-        return Container::get()
-            ->processRunner
-            ->run(
-                $command,
-                $environment,
-                $workingDirectory,
-                $tty,
-                $pty,
-                $timeout,
-                $quiet,
-                $allowFailure,
-                $notify,
-                $callback,
-                $context,
-            )
-        ;
-    } catch (\Throwable $e) {
-        throw fix_exception($e, 1);
-    }
+    return Container::get()
+        ->processRunner
+        ->run(
+            $command,
+            $environment,
+            $workingDirectory,
+            $tty,
+            $pty,
+            $timeout,
+            $quiet,
+            $allowFailure,
+            $notify,
+            $callback,
+            $context,
+        )
+    ;
 }
 
 /**
@@ -108,30 +103,26 @@ function capture(
     ?string $path = null,
 ): string {
     if ($workingDirectory && $path) {
-        throw fix_exception(new \LogicException('You cannot use both the "path" and "workingDirectory" arguments at the same time.'), 1);
+        throw new \LogicException('You cannot use both the "path" and "workingDirectory" arguments at the same time.');
     }
     if ($path) {
         trigger_deprecation('jolicode/castor', '0.15', 'The "path" argument is deprecated, use "workingDirectory" instead.');
 
         $workingDirectory = $path;
     }
 
-    try {
-        return Container::get()
-            ->processRunner
-            ->capture(
-                $command,
-                $environment,
-                $workingDirectory,
-                $timeout,
-                $allowFailure,
-                $onFailure,
-                $context,
-            )
-        ;
-    } catch (\Throwable $e) {
-        throw fix_exception($e, 2);
-    }
+    return Container::get()
+        ->processRunner
+        ->capture(
+            $command,
+            $environment,
+            $workingDirectory,
+            $timeout,
+            $allowFailure,
+            $onFailure,
+            $context,
+        )
+    ;
 }
 
 /**
@@ -148,29 +139,25 @@ function exit_code(
     ?string $path = null,
 ): int {
     if ($workingDirectory && $path) {
-        throw fix_exception(new \LogicException('You cannot use both the "path" and "workingDirectory" arguments at the same time.'));
+        throw new \LogicException('You cannot use both the "path" and "workingDirectory" arguments at the same time.');
     }
     if ($path) {
         trigger_deprecation('jolicode/castor', '0.15', 'The "path" argument is deprecated, use "workingDirectory" instead.');
 
         $workingDirectory = $path;
     }
 
-    try {
-        return Container::get()
-            ->processRunner
-            ->exitCode(
-                $command,
-                $environment,
-                $workingDirectory,
-                $timeout,
-                $quiet,
-                $context,
-            )
-        ;
-    } catch (\Throwable $e) {
-        throw fix_exception($e, 2);
-    }
+    return Container::get()
+        ->processRunner
+        ->exitCode(
+            $command,
+            $environment,
+            $workingDirectory,
+            $timeout,
+            $quiet,
+            $context,
+        )
+    ;
 }
 
 /**
@@ -491,7 +478,7 @@ function import(string $path, ?string $file = null, ?string $version = null, ?st
 function mount(string $path, ?string $namespacePrefix = null): void
 {
     if (!is_dir($path)) {
-        throw fix_exception(new \InvalidArgumentException(\sprintf('The directory "%s" does not exist.', $path)));
+        throw new \InvalidArgumentException(\sprintf('The directory "%s" does not exist.', $path));
     }
 
     Container::get()->kernel->addMount(new Mount($path, namespacePrefix: $namespacePrefix));
@@ -828,7 +815,7 @@ function guard_min_version(string $minVersion): void
 
     $minVersion = u($minVersion)->ensureStart('v')->toString();
     if (version_compare($currentVersion, $minVersion, '<')) {
-        throw fix_exception(new MinimumVersionRequirementNotMetException($minVersion, $currentVersion));
+        throw new MinimumVersionRequirementNotMetException($minVersion, $currentVersion);
     }
 }
 

diff --git a/tests/Generated/FailureFailureTest.php.err.txt b/tests/Generated/FailureFailureTest.php.err.txt
@@ -1,6 +1,6 @@
 bash: line 1: i_do_not_exist: command not found
 
-In failure.php line 13:
+In failure.php line XXXX:
 
   The command "bash -c i_do_not_exist" failed.
 

diff --git a/tests/Generated/FailureVerboseArgumentsTest.php.err.txt b/tests/Generated/FailureVerboseArgumentsTest.php.err.txt
@@ -1,4 +1,4 @@
-In failure.php line 25:
+In failure.php line XXXX:
 
   The command "bash -c i_do_not_exist" failed.
 

diff --git a/tests/Generated/FailureVerboseArgumentsTrueTest.php.err.txt b/tests/Generated/FailureVerboseArgumentsTrueTest.php.err.txt
@@ -1,4 +1,4 @@
-In functions.php line XXXX:
+In failure.php line XXXX:
 
   The command "bash -c i_do_not_exist -x -e" failed.
 

diff --git a/tests/Generated/ImportComposerNotExistingTest.php.err.txt b/tests/Generated/ImportComposerNotExistingTest.php.err.txt
@@ -1,10 +1,5 @@
-In castor.php line 5:
+In castor.php line XXXX:
 
-  Could not import "foo/bar" in ".../tests/fixtures/broken/import-composer-not-existing/castor.php" on line 5. Reason: The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
-
-
-In Composer.php line XXXX:
-
-  The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
+  Could not import "foo/bar": The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
 
 
diff --git a/tests/Generated/ImportFileNotExistTest.php.err.txt b/tests/Generated/ImportFileNotExistTest.php.err.txt
@@ -1,5 +1,5 @@
-In castor.php line 5:
+In castor.php line XXXX:
 
-  Could not import "path/to/not/existing/castor.php" in ".../tests/fixtures/broken/import-file-not-exist/castor.php" on line 5. Reason: The file "path/to/not/existing/castor.php" does not exist.
+  Could not import "path/to/not/existing/castor.php": The file "path/to/not/existing/castor.php" does not exist.
 
 
diff --git a/tests/Generated/ImportInvalidFormatTest.php.err.txt b/tests/Generated/ImportInvalidFormatTest.php.err.txt
@@ -1,10 +1,5 @@
-In castor.php line 5:
+In castor.php line XXXX:
 
-  Could not import "invalid-package-name" in ".../tests/fixtures/broken/import-invalid-format/castor.php" on line 5. Reason: The import path must be formatted like this: "composer://<organization>/<repository>".
-
-
-In Composer.php line XXXX:
-
-  The import path must be formatted like this: "composer://<organization>/<repository>".
+  Could not import "invalid-package-name": The import path must be formatted like this: "composer://<organization>/<repository>".
 
 
diff --git a/tests/Generated/ImportInvalidPackageTest.php.err.txt b/tests/Generated/ImportInvalidPackageTest.php.err.txt
@@ -1,10 +1,5 @@
-In castor.php line 5:
+In castor.php line XXXX:
 
-  Could not import "foo/bar" in ".../tests/fixtures/broken/import-invalid-package/castor.php" on line 5. Reason: The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
-
-
-In Composer.php line XXXX:
-
-  The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
+  Could not import "foo/bar": The package "foo/bar" is not installed, make sure you required it in your castor.composer.json file.
 
 
diff --git a/tests/Generated/ParallelExceptionTest.php.err.txt b/tests/Generated/ParallelExceptionTest.php.err.txt
@@ -1,20 +1,20 @@
-In parallel.php line 74:
+In parallel.php line XXXX:
 
   This is an exception
 
 
 parallel:exception
 
 
-In parallel.php line 72:
+In parallel.php line XXXX:
 
   The command "exit 1" failed.
 
 
 parallel:exception
 
 
-In ParallelRunner.php line XXXX:
+In parallel.php line XXXX:
 
   One or more exceptions were thrown in parallel.
 

diff --git a/tests/Generated/RunExceptionTest.php.err.txt b/tests/Generated/RunExceptionTest.php.err.txt
@@ -1,4 +1,4 @@
-In run.php line 64:
+In run.php line XXXX:
 
   The command "echo foo; echo bar>&2; exit 1" failed.
 

diff --git a/tests/Generated/RunExceptionVerboseTest.php.err.txt b/tests/Generated/RunExceptionVerboseTest.php.err.txt
@@ -1,4 +1,4 @@
-In run.php line 64:
+In ProcessRunner.php line XXXX:
 
   [Symfony\Component\Process\Exception\ProcessFailedException]
   The command "echo foo; echo bar>&2; exit 1" failed.
@@ -18,7 +18,7 @@ In run.php line 64:
 
 
 Exception trace:
-  at .../examples/run.php:XXXX
+  at .../src/Runner/ProcessRunner.php:XXXX
  Castor\Runner\ProcessRunner->run() at .../src/functions.php:XXXX
  Castor\run() at .../examples/run.php:XXXX
  run\exception() at .../src/Console/Command/TaskCommand.php:XXXX

diff --git a/tests/Generated/ShellBashTest.php.err.txt b/tests/Generated/ShellBashTest.php.err.txt
@@ -1,4 +1,4 @@
-In functions.php line XXXX:
+In shell.php line XXXX:
 
   TTY mode requires /dev/tty to be read/writable.
 

diff --git a/tests/Generated/ShellShTest.php.err.txt b/tests/Generated/ShellShTest.php.err.txt
@@ -1,4 +1,4 @@
-In functions.php line XXXX:
+In shell.php line XXXX:
 
   TTY mode requires /dev/tty to be read/writable.