Merge pull request #227 from php-school/error-handling

Error handling

Author: AydinHassan

app/config.php

@@ -4,6 +4,7 @@
 
 use Colors\Color;
 use PhpSchool\PhpWorkshop\Check\FileComparisonCheck;
+use PhpSchool\PhpWorkshop\ExerciseRunner\Factory\ServerRunnerFactory;
 use PhpSchool\PhpWorkshop\Listener\InitialCodeListener;
 use PhpSchool\PhpWorkshop\Listener\TearDownListener;
 use PhpSchool\PhpWorkshop\Logger\ConsoleLogger;
@@ -96,6 +97,9 @@
     'currentWorkingDirectory' => function (ContainerInterface $c) {
         return getcwd();
     },
+    'basePath' => DI\decorate(function (string $previous) {
+        return canonicalise_path($previous);
+    }),
     WorkshopType::class => WorkshopType::STANDARD(),
     Psr\Log\LoggerInterface::class => function (ContainerInterface $c) {
         $appName = $c->get('appName');
@@ -150,7 +154,7 @@
         return $colors;
     },
     OutputInterface::class => function (ContainerInterface $c) {
-        return new StdOutput($c->get(Color::class), $c->get(Terminal::class));
+        return new StdOutput($c->get(Color::class), $c->get(Terminal::class), $c->get('basePath'));
     },
 
     ExerciseRepository::class => function (ContainerInterface $c) {
@@ -166,10 +170,10 @@
 
     //Exercise Runners
     RunnerManager::class => function (ContainerInterface $c) {
-        $manager = new RunnerManager;
+        $manager = new RunnerManager();
         $manager->addFactory(new CliRunnerFactory($c->get(EventDispatcher::class)));
         $manager->addFactory(new CgiRunnerFactory($c->get(EventDispatcher::class), $c->get(RequestRenderer::class)));
-        $manager->addFactory(new CustomVerifyingRunnerFactory);
+        $manager->addFactory(new CustomVerifyingRunnerFactory());
         return $manager;
     },
 

src/Application.php

@@ -204,9 +204,9 @@ public function configure(bool $debugMode = false): ContainerInterface
             }
         }
 
-        set_error_handler(function () use ($container): bool {
-            $this->tearDown($container);
-            return false; // Use default error handler
+        //turn all notices & warnings in to exceptions
+        set_error_handler(function (int $severity, string $message, string $file, int $line) {
+            throw new \ErrorException($message, 0, $severity, $file, $line);
         });
 
         $this->container = $container;
@@ -259,12 +259,7 @@ public function run(): int
 
             $container
                 ->get(OutputInterface::class)
-                ->printError(
-                    sprintf(
-                        '%s',
-                        $message
-                    )
-                );
+                ->printException($e);
             return 1;
         }
 
@@ -280,13 +275,18 @@ public function run(): int
     private function getContainer(bool $debugMode): Container
     {
         $containerBuilder = new ContainerBuilder();
-        $containerBuilder->addDefinitions(
-            array_merge_recursive(
-                require $this->frameworkConfigLocation,
-                require $this->diConfigFile
-            )
+        $diConfig = require $this->diConfigFile;
+        $fwConfig = require $this->frameworkConfigLocation;
+
+        $fwConfig['eventListeners'] = array_merge_recursive(
+            $fwConfig['eventListeners'],
+            $diConfig['eventListeners'] ?? []
         );
 
+        unset($diConfig['eventListeners']);
+
+        $containerBuilder->addDefinitions($diConfig, $fwConfig);
+
         $containerBuilder->addDefinitions(
             [
                 'workshopTitle' => $this->workshopTitle,

src/Check/CodeExistsCheck.php

@@ -52,7 +52,7 @@ public function handleError(Error $error): void
 
         if (!$empty) {
             $openingTag = is_array($statements) && count($statements) === 1 ? $statements[0] : null;
-            $empty = $openingTag instanceof InlineHTML ? in_array($openingTag->value, ['<?php', '<?']) : false;
+            $empty = $openingTag instanceof InlineHTML && in_array($openingTag->value, ['<?php', '<?']);
         }
 
         if ($empty) {

src/Listener/InitialCodeListener.php

@@ -48,7 +48,6 @@ public function __invoke(Event $event): void
         }
 
         foreach ($exercise->getInitialCode()->getFiles() as $file) {
-            /** @var SolutionFile $file */
             if (!file_exists($this->workingDirectory . '/' . $file->getRelativePath())) {
                 copy($file->getAbsolutePath(), $this->workingDirectory . '/' . $file->getRelativePath());
                 $message = 'File successfully copied to working directory';

src/Logger/Logger.php

@@ -21,7 +21,7 @@ public function __construct(string $filePath)
 
     public function log($level, $message, array $context = []): void
     {
-        if (!file_exists($this->filePath)) {
+        if (!file_exists(dirname($this->filePath))) {
             if (!mkdir($concurrentDirectory = dirname($this->filePath), 0777, true) && !is_dir($concurrentDirectory)) {
                 throw new \RuntimeException(sprintf('Directory "%s" was not created', $concurrentDirectory));
             }

src/Output/NullOutput.php

@@ -4,13 +4,20 @@
 
 namespace PhpSchool\PhpWorkshop\Output;
 
+use Throwable;
+
 class NullOutput implements OutputInterface
 {
     public function printError(string $error): void
     {
         // noop
     }
 
+    public function printException(Throwable $exception): void
+    {
+        // noop
+    }
+
     public function write(string $content): void
     {
         // noop

src/Output/OutputInterface.php

@@ -4,6 +4,8 @@
 
 namespace PhpSchool\PhpWorkshop\Output;
 
+use Throwable;
+
 /**
  * Output interface
  */
@@ -17,6 +19,14 @@ interface OutputInterface
      */
     public function printError(string $error): void;
 
+    /**
+     * Write an Exception. Should be decorated in someway
+     * which highlights the severity.
+     *
+     * @param Throwable $exception
+     */
+    public function printException(Throwable $exception): void;
+
     /**
      * Write a string to the output.
      *

src/Output/StdOutput.php

@@ -6,6 +6,7 @@
 
 use Colors\Color;
 use PhpSchool\Terminal\Terminal;
+use Throwable;
 
 /**
  * Console output interface
@@ -22,10 +23,16 @@ class StdOutput implements OutputInterface
      */
     private $terminal;
 
-    public function __construct(Color $color, Terminal $terminal)
+    /**
+     * @var string
+     */
+    private $workshopBasePath;
+
+    public function __construct(Color $color, Terminal $terminal, string $workshopBasePath = '')
     {
         $this->color = $color;
         $this->terminal = $terminal;
+        $this->workshopBasePath = $workshopBasePath;
     }
 
     /**
@@ -41,6 +48,51 @@ public function printError(string $error): void
         echo "\n";
     }
 
+    /**
+     * @param Throwable $exception
+     */
+    public function printException(Throwable $exception): void
+    {
+        $message = $exception->getMessage();
+        if (strpos($message, $this->workshopBasePath) !== null) {
+            $message = str_replace($this->workshopBasePath, '', $message);
+        }
+
+        $file = $exception->getFile();
+        if (strpos($file, $this->workshopBasePath) !== null) {
+            $file = str_replace($this->workshopBasePath, '', $file);
+        }
+
+        $lines = [
+            sprintf("In %s line %d:", $file, $exception->getLine()),
+            sprintf("[%s (%s)]:", get_class($exception), $exception->getCode()),
+            '',
+            $message
+        ];
+
+        $length = max(array_map('strlen', $lines)) + 2;
+        $this->emptyLine();
+        $this->writeLine(' ' . $this->color->__invoke(str_repeat(' ', $length))->bg_red());
+
+        foreach ($lines as $line) {
+            $line = str_pad($line, $length - 2, ' ', STR_PAD_RIGHT);
+            $this->writeLine(' ' . $this->color->__invoke(" $line ")->bg_red()->white()->bold());
+        }
+
+        $this->writeLine(' ' . $this->color->__invoke(str_repeat(' ', $length))->bg_red());
+        $this->emptyLine();
+
+        $this->writeLine(
+            implode(
+                "\n",
+                array_map(function ($l) {
+                    return " $l";
+                },
+                explode("\n", $exception->getTraceAsString()))
+            )
+        );
+    }
+
     /**
      * Write a title section. Should be decorated in a way which makes
      * the title stand out.

src/UserStateSerializer.php

@@ -204,6 +204,8 @@ private function readJson(string $filePath): ?array
      */
     private function wipeFile(): void
     {
-        @unlink($this->path);
+        if (file_exists(sprintf('%s/%s', $this->path, static::SAVE_FILE))) {
+            unlink(sprintf('%s/%s', $this->path, static::SAVE_FILE));
+        }
     }
 }

test/ApplicationTest.php

@@ -100,7 +100,7 @@ public function testExceptionIsThrownIfResultRendererClassDoesNotExist(): void
 
     public function testTearDownEventIsFiredOnApplicationException(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing TearDown', $configFile);
 
         $container = $application->configure();
@@ -125,7 +125,7 @@ public function testTearDownEventIsFiredOnApplicationException(): void
 
     public function testLoggingExceptionDuringTearDown(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing tear down logging', $configFile);
         $exception = new \Exception('Unexpected error');
 
@@ -159,15 +159,15 @@ static function () use ($exception) {
 
     public function testConfigureReturnsSameContainerInstance(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('Testing Configure', $configFile);
 
         self::assertSame($application->configure(), $application->configure());
     }
 
     public function testDebugFlagSwitchesLoggerToConsoleLogger(): void
     {
-        $configFile = $this->getTemporaryFile('config.php', '<?php return [];');
+        $configFile = $this->getTemporaryFile('config.php', '<?php return ["basePath" => __DIR__ . "../"];');
         $application = new Application('My workshop', $configFile);
         $container = $application->configure(true);