From d6ec16d9efa65087d89c365c2ca220ec57c6562b Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 09:27:40 +0200 Subject: [PATCH 01/13] [FEATURE] Init/ErrorHandling: Add `ErrorIncident` domain model --- .../ErrorHandling/Incident/ErrorIncident.php | 38 +++++++++++++++++ .../Incident/ErrorIncidentFactory.php | 29 +++++++++++++ .../Incident/ErrorIncidentId.php | 41 +++++++++++++++++++ .../Incident/ErrorIncidentRegistry.php | 33 +++++++++++++++ .../InMemoryErrorIncidentRegistry.php | 41 +++++++++++++++++++ .../SessionPrefixedErrorIncidentFactory.php | 40 ++++++++++++++++++ 6 files changed, 222 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncident.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentFactory.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentId.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentRegistry.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/InMemoryErrorIncidentRegistry.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Incident/SessionPrefixedErrorIncidentFactory.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncident.php b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncident.php new file mode 100644 index 000000000000..0088adc823c3 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncident.php @@ -0,0 +1,38 @@ +id; + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentFactory.php b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentFactory.php new file mode 100644 index 000000000000..66e0220aac6d --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentFactory.php @@ -0,0 +1,29 @@ +value; + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentRegistry.php b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentRegistry.php new file mode 100644 index 000000000000..41fb19438355 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Incident/ErrorIncidentRegistry.php @@ -0,0 +1,33 @@ +current = $incident; + } + + public function current(): ?ErrorIncident + { + return $this->current; + } + + public function clear(): void + { + $this->current = null; + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Incident/SessionPrefixedErrorIncidentFactory.php b/components/ILIAS/Init/src/ErrorHandling/Incident/SessionPrefixedErrorIncidentFactory.php new file mode 100644 index 000000000000..e1d15d77cf3f --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Incident/SessionPrefixedErrorIncidentFactory.php @@ -0,0 +1,40 @@ +randomizer->getInt(1, 9999); + + return new ErrorIncident(new ErrorIncidentId($session_prefix . '_' . $error_number)); + } +} From 8fe7042690e756d594c13c8d7895f2c3d83ea797 Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 09:38:38 +0200 Subject: [PATCH 02/13] [FEATURE] Init/ErrorHandling: Add notification formatter --- .../Notification/ErrorIncidentUserMessage.php | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Notification/ErrorIncidentUserMessage.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Notification/ErrorIncidentUserMessage.php b/components/ILIAS/Init/src/ErrorHandling/Notification/ErrorIncidentUserMessage.php new file mode 100644 index 000000000000..a2355c1011bf --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Notification/ErrorIncidentUserMessage.php @@ -0,0 +1,67 @@ +identifier()->value(); + + if ($language !== null) { + $language->loadLanguageModule('logging'); + $message = \sprintf($language->txt('log_error_message'), $identifier); + + $mail = $this->error_settings->mail(); + if ($mail !== '') { + $message .= ' ' . \sprintf( + $language->txt('log_error_message_send_mail'), + $mail, + $identifier, + $mail + ); + } + + return $message; + } + + $message = 'Sorry, an error occured. A logfile has been created which can be identified via the code "' + . $identifier . '"'; + + $mail = $this->error_settings->mail(); + if ($mail !== '') { + $message .= ' ' . 'Please send a mail to ' . $mail . ''; + } + + return $message; + } +} From 6fb6567eb687b4b804bcb37cc4a2c39029b2032f Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 09:40:26 +0200 Subject: [PATCH 03/13] [FEATURE] Init/ErrorHandling: Add `ReportErrorIncident` use case and application ports --- .../Application/DevmodeState.php | 32 ++++++++++ .../Application/ErrorIncidentReporting.php | 32 ++++++++++ .../Application/ErrorLogDirectory.php | 29 +++++++++ .../Application/ErrorLogFileStorage.php | 39 ++++++++++++ .../ProductionOnlyErrorIncidentReporting.php | 46 ++++++++++++++ .../Application/ReportErrorIncident.php | 61 +++++++++++++++++++ 6 files changed, 239 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/DevmodeState.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ErrorIncidentReporting.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ErrorLogDirectory.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ErrorLogFileStorage.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ProductionOnlyErrorIncidentReporting.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ReportErrorIncident.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Application/DevmodeState.php b/components/ILIAS/Init/src/ErrorHandling/Application/DevmodeState.php new file mode 100644 index 000000000000..42a3891a8da3 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Application/DevmodeState.php @@ -0,0 +1,32 @@ + $sensitive_parameter_names + */ + public function write( + Inspector $inspector, + string $directory, + string $file_name, + array $sensitive_parameter_names + ): void; +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Application/ProductionOnlyErrorIncidentReporting.php b/components/ILIAS/Init/src/ErrorHandling/Application/ProductionOnlyErrorIncidentReporting.php new file mode 100644 index 000000000000..20776fb33210 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Application/ProductionOnlyErrorIncidentReporting.php @@ -0,0 +1,46 @@ +devmode_state->isActive()) { + return null; + } + + return $this->reporting->report($inspector); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Application/ReportErrorIncident.php b/components/ILIAS/Init/src/ErrorHandling/Application/ReportErrorIncident.php new file mode 100644 index 000000000000..270af6daa88e --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Application/ReportErrorIncident.php @@ -0,0 +1,61 @@ + */ + private readonly array $sensitive_parameter_names + ) { + } + + public function report(Inspector $inspector): ?ErrorIncident + { + $directory = $this->log_directory->path(); + if ($directory === '') { + return null; + } + + $incident = $this->incident_factory->create(session_id()); + $this->log_file_storage->write( + $inspector, + $directory, + $incident->identifier()->value(), + $this->sensitive_parameter_names + ); + $this->incident_registry->record($incident); + + return $incident; + } +} From 3aedc87ceeeeda002924d5e348f555ba8983849a Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 10:26:00 +0200 Subject: [PATCH 04/13] [FEATURE] Init/ErrorHandling: Add new interface to mask `ilErrorHandling` in `DelegatingHandler` --- .../ContextErrorHandlerProvider.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Application/ContextErrorHandlerProvider.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Application/ContextErrorHandlerProvider.php b/components/ILIAS/Init/src/ErrorHandling/Application/ContextErrorHandlerProvider.php new file mode 100644 index 000000000000..4a9c36464949 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Application/ContextErrorHandlerProvider.php @@ -0,0 +1,31 @@ + Date: Tue, 9 Jun 2026 13:49:17 +0200 Subject: [PATCH 05/13] [FEATURE] Init/ErrorHandling: Add DEVMODE state implementation --- .../Environment/RuntimeDevmodeState.php | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Infrastructure/Environment/RuntimeDevmodeState.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Environment/RuntimeDevmodeState.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Environment/RuntimeDevmodeState.php new file mode 100644 index 000000000000..b6860fb8fd4a --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Environment/RuntimeDevmodeState.php @@ -0,0 +1,37 @@ + Date: Tue, 9 Jun 2026 09:46:50 +0200 Subject: [PATCH 06/13] [FEATURE] Init/ErrorHandling: Add "Whoops" handler for unified error logging --- .../Whoops/RecordErrorIncidentHandler.php | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/RecordErrorIncidentHandler.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/RecordErrorIncidentHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/RecordErrorIncidentHandler.php new file mode 100644 index 000000000000..93db5c4d366f --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/RecordErrorIncidentHandler.php @@ -0,0 +1,42 @@ +error_incident_reporting->report($this->getInspector()); + + return null; + } +} From 8fd3a4216d2d72254bc4c26eb46356c033612a91 Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 09:50:00 +0200 Subject: [PATCH 07/13] [FEATURE] Init/ErrorHandling: Add outbound logging adapters for error incident reporting --- .../LoggingErrorFileStorageAdapter.php | 38 +++++++++++++++++++ .../Logging/LoggingErrorLogDirectory.php | 31 +++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php create mode 100644 components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php new file mode 100644 index 000000000000..bf2ba44f315c --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php @@ -0,0 +1,38 @@ +withExclusionList($sensitive_parameter_names); + $writer->write(); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php new file mode 100644 index 000000000000..30fab2c48bdf --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php @@ -0,0 +1,31 @@ +folder(); + } +} From 3a10c964c3905c36fddc2a090278d104d3433b1a Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 10:13:13 +0200 Subject: [PATCH 08/13] [FEATURE] Init/ErrorHandling: Move error handlers to `src` --- .../Whoops/DelegatingHandler.php} | 17 +++++----- .../Whoops/PlainTextHandler.php} | 31 +++++++------------ .../Whoops/SoapExceptionHandler.php} | 21 +++++++++---- .../Infrastructure/Whoops/TestingHandler.php} | 5 +-- 4 files changed, 40 insertions(+), 34 deletions(-) rename components/ILIAS/Init/{classes/ErrorHandling/class.ilDelegatingHandler.php => src/ErrorHandling/Infrastructure/Whoops/DelegatingHandler.php} (89%) rename components/ILIAS/Init/{classes/ErrorHandling/class.ilPlainTextHandler.php => src/ErrorHandling/Infrastructure/Whoops/PlainTextHandler.php} (87%) rename components/ILIAS/Init/{classes/ErrorHandling/class.ilSoapExceptionHandler.php => src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php} (81%) rename components/ILIAS/Init/{classes/ErrorHandling/class.ilTestingHandler.php => src/ErrorHandling/Infrastructure/Whoops/TestingHandler.php} (89%) diff --git a/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/DelegatingHandler.php similarity index 89% rename from components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/DelegatingHandler.php index 221a908a9030..44884b1cc182 100755 --- a/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/DelegatingHandler.php @@ -18,6 +18,9 @@ declare(strict_types=1); +namespace ILIAS\Init\ErrorHandling\Infrastructure\Whoops; + +use ILIAS\Init\ErrorHandling\Application\ContextErrorHandlerProvider; use Whoops\Handler\Handler; use Whoops\Handler\HandlerInterface; @@ -30,9 +33,8 @@ * workaround. * This class is not ment to be extended, as the definition of error handlers should be handled in one place * in ilErrorHandling, so this class acts rather dump and asks ilErrorHandling for a handler. - * @author Richard Klees */ -final class ilDelegatingHandler extends Handler +final class DelegatingHandler extends Handler { private ?HandlerInterface $current_handler = null; @@ -40,7 +42,7 @@ final class ilDelegatingHandler extends Handler * @param list $sensitive_data */ public function __construct( - private readonly ilErrorHandling $error_handling, + private readonly ContextErrorHandlerProvider $error_handling, private readonly array $sensitive_data = [] ) { } @@ -48,15 +50,15 @@ public function __construct( private function hideSensitiveData(array $key_value_pairs): array { foreach ($key_value_pairs as $key => &$value) { - if (is_array($value)) { + if (\is_array($value)) { $value = $this->hideSensitiveData($value); } - if (is_string($value) && in_array($key, $this->sensitive_data, true)) { + if (\is_string($value) && \in_array($key, $this->sensitive_data, true)) { $value = 'REMOVED FOR SECURITY'; } - if ($key === 'PHPSESSID' && is_string($value)) { + if ($key === 'PHPSESSID' && \is_string($value)) { $value = substr($value, 0, 5) . ' (SHORTENED FOR SECURITY)'; } @@ -85,7 +87,7 @@ private function hideSensitiveData(array $key_value_pairs): array */ public function handle(): ?int { - if (defined('IL_INITIAL_WD')) { + if (\defined('IL_INITIAL_WD')) { chdir(IL_INITIAL_WD); } @@ -103,6 +105,7 @@ public function handle(): ?int $this->current_handler->setRun($this->getRun()); $this->current_handler->setException($this->getException()); $this->current_handler->setInspector($this->getInspector()); + return $this->current_handler->handle(); } diff --git a/components/ILIAS/Init/classes/ErrorHandling/class.ilPlainTextHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/PlainTextHandler.php similarity index 87% rename from components/ILIAS/Init/classes/ErrorHandling/class.ilPlainTextHandler.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/PlainTextHandler.php index ef66bf13c729..bfc6e0f1997c 100755 --- a/components/ILIAS/Init/classes/ErrorHandling/class.ilPlainTextHandler.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/PlainTextHandler.php @@ -18,16 +18,19 @@ declare(strict_types=1); +namespace ILIAS\Init\ErrorHandling\Infrastructure\Whoops; + +use Throwable; use Whoops\Exception\Formatter; +use Whoops\Handler\PlainTextHandler as WhoopsPlainTextHandler; /** * A Whoops error handler that prints the same content as the PrettyPageHandler but as plain text. * This is used for better coexistence with xdebug, see #16627. - * @author Richard Klees */ -class ilPlainTextHandler extends \Whoops\Handler\PlainTextHandler +class PlainTextHandler extends WhoopsPlainTextHandler { - protected const KEY_SPACE = 25; + protected const int KEY_SPACE = 25; /** @var list */ private array $exclusion_list = []; @@ -39,6 +42,7 @@ public function withExclusionList(array $exclusion_list): self { $clone = clone $this; $clone->exclusion_list = $exclusion_list; + return $clone; } @@ -54,18 +58,15 @@ public function generateResponse(): string protected function getSimpleExceptionOutput(Throwable $exception): string { - return sprintf( + return \sprintf( '%s: %s in file %s on line %d', - get_class($exception), + $exception::class, $exception->getMessage(), $exception->getFile(), $exception->getLine() ); } - /** - * Get a short info about the exception. - */ protected function getPlainTextExceptionOutput(bool $with_previous = true): string { $message = Formatter::formatExceptionPlain($this->getInspector()); @@ -82,20 +83,15 @@ protected function getPlainTextExceptionOutput(bool $with_previous = true): stri return $message; } - /** - * Get the header for the page. - */ protected function tablesContent(): string { $ret = ''; foreach ($this->tables() as $title => $content) { $ret .= "\n\n-- $title --\n\n"; - if (count($content) > 0) { + if ($content !== []) { foreach ($content as $key => $value) { $key = str_pad((string) $key, self::KEY_SPACE); - // indent multiline values, first print_r, split in lines, - // indent all but first line, then implode again. $first = true; $indentation = str_pad('', self::KEY_SPACE); $value = implode( @@ -106,6 +102,7 @@ static function ($line) use (&$first, $indentation): string { $first = false; return $line; } + return $indentation . $line; }, explode("\n", print_r($value, true)) @@ -122,9 +119,6 @@ static function ($line) use (&$first, $indentation): string { return $this->stripNullBytes($ret); } - /** - * Get the tables that should be rendered. - */ protected function tables(): array { $post = $_POST; @@ -170,8 +164,7 @@ private function hideSensitiveData(array $super_global): array */ private function shortenPHPSessionId(array $server): array { - $cookie_content = $server['HTTP_COOKIE']; - $cookie_content = explode(';', $cookie_content); + $cookie_content = explode(';', $server['HTTP_COOKIE']); foreach ($cookie_content as $key => $content) { $content_array = explode('=', $content); diff --git a/components/ILIAS/Init/classes/ErrorHandling/class.ilSoapExceptionHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php similarity index 81% rename from components/ILIAS/Init/classes/ErrorHandling/class.ilSoapExceptionHandler.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php index 6efb7c69662e..f5fb1265ed07 100644 --- a/components/ILIAS/Init/classes/ErrorHandling/class.ilSoapExceptionHandler.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php @@ -18,15 +18,24 @@ declare(strict_types=1); -class ilSoapExceptionHandler extends \Whoops\Handler\Handler +namespace ILIAS\Init\ErrorHandling\Infrastructure\Whoops; + +use Throwable; +use Whoops\Exception\Formatter; +use Whoops\Handler\Handler; + +/** + * Whoops handler that renders SOAP fault responses for SOAP POST requests. + */ +final class SoapExceptionHandler extends Handler { private function buildFaultString(): string { - if (!defined('DEVMODE') || DEVMODE !== 1) { + if (!\defined('DEVMODE') || DEVMODE !== 1) { return htmlspecialchars($this->getInspector()->getException()->getMessage()); } - $fault_string = \Whoops\Exception\Formatter::formatExceptionPlain($this->getInspector()); + $fault_string = Formatter::formatExceptionPlain($this->getInspector()); $exception = $this->getInspector()->getException(); $previous = $exception->getPrevious(); while ($previous) { @@ -39,9 +48,9 @@ private function buildFaultString(): string private function getSimpleExceptionOutput(Throwable $exception): string { - return sprintf( + return \sprintf( '%s: %s in file %s on line %d', - get_class($exception), + $exception::class, $exception->getMessage(), $exception->getFile(), $exception->getLine() @@ -52,7 +61,7 @@ public function handle(): ?int { echo $this->toXml(); - return \Whoops\Handler\Handler::QUIT; + return Handler::QUIT; } private function toXml(): string diff --git a/components/ILIAS/Init/classes/ErrorHandling/class.ilTestingHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/TestingHandler.php similarity index 89% rename from components/ILIAS/Init/classes/ErrorHandling/class.ilTestingHandler.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/TestingHandler.php index 9fed81e5416a..89f5bc4d47f1 100755 --- a/components/ILIAS/Init/classes/ErrorHandling/class.ilTestingHandler.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/TestingHandler.php @@ -18,13 +18,14 @@ declare(strict_types=1); +namespace ILIAS\Init\ErrorHandling\Infrastructure\Whoops; + /** * A Whoops error handler for testing. * This yields the same output as the plain text handler, but prints a nice message to the tester on top of * the page. - * @author Richard Klees */ -class ilTestingHandler extends ilPlainTextHandler +final class TestingHandler extends PlainTextHandler { public function generateResponse(): string { From 707c04d0549ffb80fda3997fafa7b4c00744312d Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 12:39:55 +0200 Subject: [PATCH 09/13] [FEATURE] Init/ErrorHandling: Provide incident id in SOAP-based handler --- .../Whoops/SoapExceptionHandler.php | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php index f5fb1265ed07..f4ccf3e838c5 100644 --- a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Whoops/SoapExceptionHandler.php @@ -20,6 +20,8 @@ namespace ILIAS\Init\ErrorHandling\Infrastructure\Whoops; +use ILIAS\Init\ErrorHandling\Application\DevmodeState; +use ILIAS\Init\ErrorHandling\Incident\ErrorIncidentRegistry; use Throwable; use Whoops\Exception\Formatter; use Whoops\Handler\Handler; @@ -29,18 +31,30 @@ */ final class SoapExceptionHandler extends Handler { + public function __construct( + private readonly ErrorIncidentRegistry $incident_registry, + private readonly DevmodeState $devmode_state + ) { + } + private function buildFaultString(): string { - if (!\defined('DEVMODE') || DEVMODE !== 1) { - return htmlspecialchars($this->getInspector()->getException()->getMessage()); + $incident = $this->incident_registry->current(); + + if ($this->devmode_state->isActive()) { + $fault_string = Formatter::formatExceptionPlain($this->getInspector()); + $exception = $this->getInspector()->getException(); + $previous = $exception->getPrevious(); + while ($previous) { + $fault_string .= "\n\nCaused by\n" . $this->getSimpleExceptionOutput($previous); + $previous = $previous->getPrevious(); + } + } else { + $fault_string = $this->getInspector()->getException()->getMessage(); } - $fault_string = Formatter::formatExceptionPlain($this->getInspector()); - $exception = $this->getInspector()->getException(); - $previous = $exception->getPrevious(); - while ($previous) { - $fault_string .= "\n\nCaused by\n" . $this->getSimpleExceptionOutput($previous); - $previous = $previous->getPrevious(); + if ($incident !== null) { + $fault_string .= "\n\n (incident code: " . $incident->identifier()->value() . ')'; } return htmlspecialchars($fault_string); From 1888f60a77a08cfcf4bb8d9250d7ecb1d33dffb2 Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 11:11:57 +0200 Subject: [PATCH 10/13] [FEATURE] Init/ErrorHandling: Unify error logging --- .../Init/classes/class.ilErrorHandling.php | 74 ++++++++++--------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/components/ILIAS/Init/classes/class.ilErrorHandling.php b/components/ILIAS/Init/classes/class.ilErrorHandling.php index 7676978ba4c1..33807ca5b0bd 100755 --- a/components/ILIAS/Init/classes/class.ilErrorHandling.php +++ b/components/ILIAS/Init/classes/class.ilErrorHandling.php @@ -18,6 +18,9 @@ declare(strict_types=1); +use ILIAS\Init\ErrorHandling\Infrastructure\Whoops as ErrorHandlers; +use ILIAS\Init\ErrorHandling\Infrastructure\Logging as ErrorLogging; +use ILIAS\Init\ErrorHandling; use Whoops\Run; use Whoops\RunInterface; use Whoops\Handler\PrettyPageHandler; @@ -34,7 +37,7 @@ * @todo when an error occured and clicking the back button to return to previous page the referer-var in session is deleted -> server error * @todo This class is a candidate for a singleton. initHandlers could only be called once per process anyways, as it checks for static $handlers_registered. */ -class ilErrorHandling +class ilErrorHandling implements ErrorHandling\Application\ContextErrorHandlerProvider { /** @var list */ private const array SENSTIVE_PARAMETER_NAMES = [ @@ -55,6 +58,8 @@ class ilErrorHandling protected ?RunInterface $whoops; protected string $message; + protected ErrorHandling\Incident\ErrorIncidentRegistry $error_incident_registry; + protected ErrorHandling\Application\DevmodeState $devmode_state; /** Error level 1: exit application immedietly */ public int $FATAL = 1; /** Error level 2: show warning page */ @@ -67,6 +72,8 @@ public function __construct() $this->FATAL = 1; $this->WARNING = 2; $this->MESSAGE = 3; + $this->error_incident_registry = new ErrorHandling\Incident\InMemoryErrorIncidentRegistry(); + $this->devmode_state = new ErrorHandling\Infrastructure\Environment\RuntimeDevmodeState(); $this->initWhoopsHandlers(); @@ -89,10 +96,26 @@ protected function initWhoopsHandlers(): void $runtime = $this->getRuntime(); $this->whoops = $this->getWhoops(); - $this->whoops->pushHandler(new ilDelegatingHandler($this, self::SENSTIVE_PARAMETER_NAMES)); + $this->whoops->pushHandler( + new ErrorHandlers\DelegatingHandler($this, self::SENSTIVE_PARAMETER_NAMES) + ); if ($runtime->shouldLogErrors()) { $this->whoops->pushHandler($this->loggingHandler()); } + $this->whoops->pushHandler( + new ErrorHandlers\RecordErrorIncidentHandler( + new ErrorHandling\Application\ProductionOnlyErrorIncidentReporting( + new ErrorHandling\Application\ReportErrorIncident( + new ErrorLogging\LoggingErrorLogDirectory(), + new ErrorLogging\LoggingErrorFileStorageAdapter(), + new ErrorHandling\Incident\SessionPrefixedErrorIncidentFactory(), + $this->error_incident_registry, + self::SENSTIVE_PARAMETER_NAMES + ), + $this->devmode_state + ) + ) + ); $this->whoops->register(); self::$whoops_handlers_registered = true; @@ -106,7 +129,10 @@ public function getHandler(): HandlerInterface { if (ilContext::getType() === ilContext::CONTEXT_SOAP && strcasecmp($_SERVER['REQUEST_METHOD'] ?? '', 'post') === 0) { - return new ilSoapExceptionHandler(); + return new ErrorHandlers\SoapExceptionHandler( + $this->error_incident_registry, + $this->devmode_state + ); } // TODO: There might be more specific execution contexts (WebDAV, REST, etc.) that need specific error handling. @@ -222,7 +248,7 @@ protected function getWhoops(): RunInterface protected function isDevmodeActive(): bool { - return defined('DEVMODE') && (int) DEVMODE === 1; + return $this->devmode_state->isActive(); } protected function defaultHandler(): HandlerInterface @@ -230,40 +256,18 @@ protected function defaultHandler(): HandlerInterface return new CallbackHandler(function ($exception, Inspector $inspector, Run $run) { global $DIC; - $logger = ilLoggingErrorSettings::getInstance(); - $message = 'Sorry, an error occured.'; if ($DIC->isDependencyAvailable('language')) { $DIC->language()->loadLanguageModule('logging'); $message = $DIC->language()->txt('error_sry_error'); } - if (!empty($logger->folder())) { - $session_id = substr(session_id(), 0, 5); - $r = new \Random\Randomizer(); - $err_num = $r->getInt(1, 9999); - $file_name = $session_id . '_' . $err_num; - - $lwriter = new ilLoggingErrorFileStorage($inspector, $logger->folder(), $file_name); - $lwriter = $lwriter->withExclusionList(self::SENSTIVE_PARAMETER_NAMES); - $lwriter->write(); - - if ($DIC->isDependencyAvailable('language')) { - $message = sprintf($DIC->language()->txt('log_error_message'), $file_name); - if ($logger->mail()) { - $message .= ' ' . sprintf( - $DIC->language()->txt('log_error_message_send_mail'), - $logger->mail(), - $file_name, - $logger->mail() - ); - } - } else { - $message = 'Sorry, an error occured. A logfile has been created which can be identified via the code "' . $file_name . '"'; - if ($logger->mail()) { - $message .= ' ' . 'Please send a mail to ' . $logger->mail() . ''; - } - } + $incident = $this->error_incident_registry->current(); + if ($incident !== null) { + $language = $DIC->isDependencyAvailable('language') ? $DIC->language() : null; + $message = new ErrorHandling\Notification\ErrorIncidentUserMessage( + ilLoggingErrorSettings::getInstance() + )->format($incident, $language); } if ($DIC->isDependencyAvailable('ui') && isset($DIC['tpl']) && $DIC->isDependencyAvailable('ctrl')) { @@ -283,10 +287,12 @@ protected function devmodeHandler(): HandlerInterface switch (ERROR_HANDLER) { case 'TESTING': - return (new ilTestingHandler())->withExclusionList(self::SENSTIVE_PARAMETER_NAMES); + return new ErrorHandlers\TestingHandler() + ->withExclusionList(self::SENSTIVE_PARAMETER_NAMES); case 'PLAIN_TEXT': - return (new ilPlainTextHandler())->withExclusionList(self::SENSTIVE_PARAMETER_NAMES); + return new ErrorHandlers\PlainTextHandler() + ->withExclusionList(self::SENSTIVE_PARAMETER_NAMES); case 'PRETTY_PAGE': // fallthrough From 4d4412b1770a2a24ef73ad3d03fecc93d57bdeae Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 11:07:35 +0200 Subject: [PATCH 11/13] [FEATURE] Init/ErrorHandling: Add error handling tests --- .../ErrorHandling/ErrorIncidentIdTest.php | 40 ++++++ .../tests/ErrorHandling/ErrorIncidentTest.php | 35 ++++++ .../ErrorIncidentUserMessageTest.php | 59 +++++++++ .../InMemoryErrorIncidentRegistryTest.php | 54 ++++++++ .../LoggingErrorFileStorageAdapterTest.php | 62 ++++++++++ ...oductionOnlyErrorIncidentReportingTest.php | 92 ++++++++++++++ .../RecordErrorIncidentHandlerTest.php | 39 ++++++ .../ErrorHandling/ReportErrorIncidentTest.php | 116 ++++++++++++++++++ ...essionPrefixedErrorIncidentFactoryTest.php | 45 +++++++ .../SoapExceptionHandlerTest.php | 88 +++++++++++++ 10 files changed, 630 insertions(+) create mode 100644 components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentIdTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/InMemoryErrorIncidentRegistryTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/ProductionOnlyErrorIncidentReportingTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/RecordErrorIncidentHandlerTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/SessionPrefixedErrorIncidentFactoryTest.php create mode 100644 components/ILIAS/Init/tests/ErrorHandling/SoapExceptionHandlerTest.php diff --git a/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentIdTest.php b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentIdTest.php new file mode 100644 index 000000000000..0096d2990622 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentIdTest.php @@ -0,0 +1,40 @@ +value()); + } + + public function testRejectsEmptyValue(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Error incident identifier must not be empty.'); + + new ErrorIncidentId(''); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentTest.php b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentTest.php new file mode 100644 index 000000000000..78b56e5c1249 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentTest.php @@ -0,0 +1,35 @@ +identifier()); + self::assertSame('abc_1234', $incident->identifier()->value()); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php new file mode 100644 index 000000000000..134c7df9a2be --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php @@ -0,0 +1,59 @@ +createMock(ilLoggingErrorSettings::class); + $settings->method('mail')->willReturn('admin@example.org'); + + $message_formatter = new ErrorIncidentUserMessage($settings); + $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), null); + + self::assertStringContainsString('abc_12', $message); + self::assertStringContainsString('admin@example.org', $message); + } + + public function testFormatsLocalizedMessageWithLanguage(): void + { + $settings = $this->createMock(ilLoggingErrorSettings::class); + $settings->method('mail')->willReturn(''); + + $language = $this->createMock(ilLanguage::class); + $language->expects($this->once())->method('loadLanguageModule')->with('logging'); + $language->method('txt')->willReturnCallback( + static fn(string $key): string => match ($key) { + 'log_error_message' => 'Logged error %s', + default => $key, + } + ); + + $message_formatter = new ErrorIncidentUserMessage($settings); + $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), $language); + + self::assertSame('Logged error abc_12', $message); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/InMemoryErrorIncidentRegistryTest.php b/components/ILIAS/Init/tests/ErrorHandling/InMemoryErrorIncidentRegistryTest.php new file mode 100644 index 000000000000..48c9b9370169 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/InMemoryErrorIncidentRegistryTest.php @@ -0,0 +1,54 @@ +current()); + } + + public function testRecordsAndReturnsCurrentIncident(): void + { + $registry = new InMemoryErrorIncidentRegistry(); + $incident = new ErrorIncident(new ErrorIncidentId('abc_99')); + + $registry->record($incident); + + self::assertSame($incident, $registry->current()); + } + + public function testClearRemovesCurrentIncident(): void + { + $registry = new InMemoryErrorIncidentRegistry(); + $registry->record(new ErrorIncident(new ErrorIncidentId('abc_99'))); + + $registry->clear(); + + self::assertNull($registry->current()); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php b/components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php new file mode 100644 index 000000000000..09332741b8c1 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php @@ -0,0 +1,62 @@ +skipIfVfsStreamNotAvailable(); + + vfsStream::setup(); + vfsStream::create([ + 'errors' => [], + ]); + + $log_directory = vfsStream::url('root/errors'); + $log_file = vfsStream::url('root/errors/abcde_42.log'); + $inspector = new Inspector(new RuntimeException('adapter test')); + + $adapter = new LoggingErrorFileStorageAdapter(); + $adapter->write( + $inspector, + $log_directory, + 'abcde_42', + ['password'] + ); + + self::assertFileExists($log_file); + self::assertStringContainsString('adapter test', (string) file_get_contents($log_file)); + } + + private function skipIfVfsStreamNotAvailable(): void + { + if (!class_exists(vfsStreamWrapper::class)) { + self::markTestSkipped( + 'vfsStream (https://github.com/bovigo/vfsStream) is required for virtual filesystem tests.' + ); + } + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/ProductionOnlyErrorIncidentReportingTest.php b/components/ILIAS/Init/tests/ErrorHandling/ProductionOnlyErrorIncidentReportingTest.php new file mode 100644 index 000000000000..afe48058eef2 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/ProductionOnlyErrorIncidentReportingTest.php @@ -0,0 +1,92 @@ +createMock(ErrorIncidentReporting::class); + $inner->expects($this->once()) + ->method('report') + ->with($inspector) + ->willReturn($incident); + + $reporting = new ProductionOnlyErrorIncidentReporting($inner, $this->devmodeState(false)); + + $result = $reporting->report($inspector); + + self::assertSame($incident, $result); + } + + public function testSkipsReportingWhenDevmodeIsActive(): void + { + $inner = $this->createMock(ErrorIncidentReporting::class); + $inner->expects($this->never())->method('report'); + + $reporting = new ProductionOnlyErrorIncidentReporting($inner, $this->devmodeState(true)); + + $result = $reporting->report(new Inspector(new RuntimeException('test'))); + + self::assertNull($result); + } + + public function testEvaluatesDevmodeLazilyOnEveryReport(): void + { + $inspector = new Inspector(new RuntimeException('test')); + $incident = new ErrorIncident(new ErrorIncidentId('abc_12')); + + $inner = $this->createStub(ErrorIncidentReporting::class); + $inner->method('report')->willReturn($incident); + + $devmode = $this->devmodeState(true); + $reporting = new ProductionOnlyErrorIncidentReporting($inner, $devmode); + + self::assertNull($reporting->report($inspector)); + + $devmode->is_active = false; + + self::assertSame($incident, $reporting->report($inspector)); + } + + private function devmodeState(bool $is_active): DevmodeState + { + return new class ($is_active) implements DevmodeState { + public function __construct(public bool $is_active) + { + } + + public function isActive(): bool + { + return $this->is_active; + } + }; + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/RecordErrorIncidentHandlerTest.php b/components/ILIAS/Init/tests/ErrorHandling/RecordErrorIncidentHandlerTest.php new file mode 100644 index 000000000000..0b3ca8aefbfe --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/RecordErrorIncidentHandlerTest.php @@ -0,0 +1,39 @@ +createMock(ErrorIncidentReporting::class); + $inspector = new Inspector(new RuntimeException('test')); + $reporting->expects($this->once())->method('report')->with($inspector)->willReturn(null); + + $handler = new RecordErrorIncidentHandler($reporting); + $handler->setInspector($inspector); + + self::assertNull($handler->handle()); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php b/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php new file mode 100644 index 000000000000..d9fe44baa31b --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php @@ -0,0 +1,116 @@ +createMock(ErrorLogFileStorage::class); + $storage->expects($this->never())->method('write'); + + $registry = new InMemoryErrorIncidentRegistry(); + $report = new ReportErrorIncident( + new readonly class () implements ErrorLogDirectory { + public function path(): string + { + return ''; + } + }, + $storage, + new SessionPrefixedErrorIncidentFactory(), + $registry, + ['password'] + ); + + $result = $report->report(new Inspector(new RuntimeException('test'))); + + self::assertNull($result); + self::assertNull($registry->current()); + } + + public function testWritesLogFileAndRecordsIncident(): void + { + $this->skipIfVfsStreamNotAvailable(); + + vfsStream::setup(); + vfsStream::create([ + 'errors' => [], + ]); + + $log_directory = vfsStream::url('root/errors'); + $log_file = vfsStream::url('root/errors/abcde_42.log'); + $inspector = new Inspector(new RuntimeException('test')); + $incident = new ErrorIncident(new ErrorIncidentId('abcde_42')); + + $incident_factory = $this->createMock(ErrorIncidentFactory::class); + $incident_factory->expects($this->once()) + ->method('create') + ->willReturn($incident); + + $registry = new InMemoryErrorIncidentRegistry(); + $report = new ReportErrorIncident( + new readonly class ($log_directory) implements ErrorLogDirectory { + public function __construct( + private string $path + ) { + } + + public function path(): string + { + return $this->path; + } + }, + new LoggingErrorFileStorageAdapter(), + $incident_factory, + $registry, + ['password'] + ); + + $result = $report->report($inspector); + + self::assertSame($incident, $result); + self::assertSame($incident, $registry->current()); + self::assertFileExists($log_file); + self::assertStringContainsString('test', (string) file_get_contents($log_file)); + } + + private function skipIfVfsStreamNotAvailable(): void + { + if (!class_exists(vfsStreamWrapper::class)) { + self::markTestSkipped( + 'vfsStream (https://github.com/bovigo/vfsStream) is required for virtual filesystem tests.' + ); + } + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/SessionPrefixedErrorIncidentFactoryTest.php b/components/ILIAS/Init/tests/ErrorHandling/SessionPrefixedErrorIncidentFactoryTest.php new file mode 100644 index 000000000000..dce430a0a33f --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/SessionPrefixedErrorIncidentFactoryTest.php @@ -0,0 +1,45 @@ +create('abcdef0123456789'); + + self::assertSame('abcde_9997', $incident->identifier()->value()); + } + + public function testCreatesIdentifierWhenSessionIdIsEmpty(): void + { + $engine = new \Random\Engine\Mt19937(99); + $factory = new SessionPrefixedErrorIncidentFactory(new \Random\Randomizer($engine)); + + $incident = $factory->create(''); + + self::assertSame('_3172', $incident->identifier()->value()); + } +} diff --git a/components/ILIAS/Init/tests/ErrorHandling/SoapExceptionHandlerTest.php b/components/ILIAS/Init/tests/ErrorHandling/SoapExceptionHandlerTest.php new file mode 100644 index 000000000000..69b344d819b9 --- /dev/null +++ b/components/ILIAS/Init/tests/ErrorHandling/SoapExceptionHandlerTest.php @@ -0,0 +1,88 @@ +record(new ErrorIncident(new ErrorIncidentId('abc_12'))); + + $handler = new SoapExceptionHandler($registry, $this->devmodeState(false)); + $handler->setInspector(new Inspector(new RuntimeException('internal soap failure'))); + + ob_start(); + $handler->handle(); + $output = (string) ob_get_clean(); + + self::assertStringContainsString('internal soap failure', $output); + self::assertStringContainsString('abc_12', $output); + } + + public function testFallsBackToExceptionMessageWithoutIncident(): void + { + $handler = new SoapExceptionHandler(new InMemoryErrorIncidentRegistry(), $this->devmodeState(false)); + $handler->setInspector(new Inspector(new RuntimeException('internal soap failure'))); + + ob_start(); + $handler->handle(); + $output = (string) ob_get_clean(); + + self::assertStringContainsString('internal soap failure', $output); + } + + public function testAppendsIncidentReferenceInDevmodeFaultString(): void + { + $registry = new InMemoryErrorIncidentRegistry(); + $registry->record(new ErrorIncident(new ErrorIncidentId('abc_12'))); + + $handler = new SoapExceptionHandler($registry, $this->devmodeState(true)); + $handler->setInspector(new Inspector(new RuntimeException('internal soap failure'))); + + ob_start(); + $handler->handle(); + $output = (string) ob_get_clean(); + + self::assertStringContainsString('internal soap failure', $output); + self::assertStringContainsString('abc_12', $output); + } + + private function devmodeState(bool $is_active): DevmodeState + { + return new class ($is_active) implements DevmodeState { + public function __construct(private bool $is_active) + { + } + + public function isActive(): bool + { + return $this->is_active; + } + }; + } +} From 4020add60a922bdc2f1c6d07a9c5a4f3076c34c1 Mon Sep 17 00:00:00 2001 From: mjansen Date: Tue, 9 Jun 2026 11:23:47 +0200 Subject: [PATCH 12/13] [FEATURE] Init/ErrorHandling: Update ROADMAP.md and README.md --- .../{classes => src}/ErrorHandling/README.md | 31 +++++++++++++++++-- .../ILIAS/Init/src/ErrorHandling/ROADMAP.md | 29 +++++++---------- 2 files changed, 39 insertions(+), 21 deletions(-) rename components/ILIAS/Init/{classes => src}/ErrorHandling/README.md (54%) diff --git a/components/ILIAS/Init/classes/ErrorHandling/README.md b/components/ILIAS/Init/src/ErrorHandling/README.md similarity index 54% rename from components/ILIAS/Init/classes/ErrorHandling/README.md rename to components/ILIAS/Init/src/ErrorHandling/README.md index cef25f90620d..e4d4d7b0eca7 100644 --- a/components/ILIAS/Init/classes/ErrorHandling/README.md +++ b/components/ILIAS/Init/src/ErrorHandling/README.md @@ -1,8 +1,33 @@ -# Error Responders +# Error Handling -This package provides responders for rendering HTTP error pages in ILIAS. +This package covers HTTP error responses and exception logging for ILIAS. -## When to use which responder +## Error incidents and log files + +If a dedicated error log folder is configured, uncaught exceptions are written +to a file in that folder. The user-facing error message references the same +identifier as the file name (for example `abcde_1234`), so reports can be matched +to log files on disk. + +The identifier is represented as an `ErrorIncident` and kept for the current +request in an `ErrorIncidentRegistry`. That way the handler writing the log file +and the handler building the response message share one value. + +`ReportErrorIncident` performs the actual reporting. It is invoked from +`RecordErrorIncidentHandler`, which is registered in the Whoops chain before the +response handlers run. Implementation code is under `Init/src/ErrorHandling/` +(`Incident`, `Application`, `Notification`, `Infrastructure`). + +## Whoops handler chain + +`ilErrorHandling` registers handlers in reverse order (the last pushed handler +runs first): + +1. `RecordErrorIncidentHandler` — writes the dedicated log file when configured +2. `loggingHandler()` — application log and `error_log()` where enabled +3. `DelegatingHandler` — selects the response handler (production, SOAP, devmode, …) + +## When to use which HTTP responder - **ErrorPageResponder** (`Http\ErrorPageResponder`): Use when the DI container and all ILIAS services (UI, language, HTTP, etc.) are available. Renders a full ILIAS page with a UI-Framework MessageBox and optional back button. Use for expected errors (e.g. routing failures, access denied) that should be shown as a proper HTML page. diff --git a/components/ILIAS/Init/src/ErrorHandling/ROADMAP.md b/components/ILIAS/Init/src/ErrorHandling/ROADMAP.md index 3770ffb90e59..47178e21b1e3 100644 --- a/components/ILIAS/Init/src/ErrorHandling/ROADMAP.md +++ b/components/ILIAS/Init/src/ErrorHandling/ROADMAP.md @@ -49,24 +49,17 @@ In almost all cases this redirect is unnecessary: ### Unified log file reporting for all handlers -**Current behaviour** - -Only the **default handler** (production) writes exceptions to a dedicated log -file (via `ilLoggingErrorFileStorage`) when configured. Other handlers (e.g., -SOAP, testing, devmode handlers) do not write to that log file. +**Done** (ILIAS 12). -**Goal** +Previously only the production default handler wrote exceptions to the dedicated +log file via `ilLoggingErrorFileStorage`. SOAP, testing, and devmode handlers did +not. -- Make the ability to report an exception to the dedicated log file available to - **all** Whoops handlers (default, SOAP, testing, devmode, etc.), not only the - default handler. -- Ensure a consistent reporting path: whenever an exception is handled and - logging is enabled, it can be written to the configured log file regardless - of which handler rendered the response. - -**Outcome** +Log file writing now happens in `RecordErrorIncidentHandler`, which runs for every +handled exception before the response handler is chosen. It calls +`ReportErrorIncident` and stores the incident in `ErrorIncidentRegistry`. The +production handler reads that value when it builds the message for the user, so +the code in the UI matches the log file name. -- Administrators and developers get a single, consistent log of reported exceptions - across all entry points and handler types. -- Easier auditing and debugging when errors occur in SOAP, tests, or other - contexts that today do not use the dedicated log file. +Details are documented in `README.md` and implemented under +`Init/src/ErrorHandling/`. From a5f30a68e7f2e17cb6340c991637ccb6acd7472d Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Wed, 22 Jul 2026 11:51:19 +0200 Subject: [PATCH 13/13] Init/ErrorHandling: absorb error log handling from Logging --- .../Init/classes/class.ilErrorHandling.php | 9 +- components/ILIAS/Init/service.xml | 3 + .../Logging/ContentProcessor.php} | 123 ++++++------- ...rFileStorageAdapter.php => FileWriter.php} | 22 ++- .../Infrastructure/Logging/LazySettings.php | 52 ++++++ .../CleanUp/CleanUpOldErrorLogsJob.php | 142 +++++++++++++++ .../Logging/CleanUp/Settings.php | 50 ++++++ .../src/ErrorHandling/Logging/FileHandler.php | 97 +++++++++++ .../src/ErrorHandling/Logging/Settings.php | 36 ++++ .../Logging/SettingsInterface.php | 26 +++ .../Notification/ErrorIncidentUserMessage.php | 19 +- .../ErrorHandling/Notification/Settings.php | 46 +++++ .../SettingsInterface.php} | 13 +- .../ErrorIncidentUserMessageTest.php | 20 +-- ...est.php => ErrorLoggingFileWriterTest.php} | 14 +- .../ErrorHandling/ReportErrorIncidentTest.php | 6 +- .../classes/class.ilObjLoggingSettingsGUI.php | 42 ++--- .../class.ilLoggerCronCleanErrorFiles.php | 163 ------------------ .../error/class.ilLoggingErrorSettings.php | 95 ---------- components/ILIAS/Logging/service.xml | 3 - 20 files changed, 575 insertions(+), 406 deletions(-) rename components/ILIAS/{Logging/classes/error/class.ilLoggingErrorFileStorage.php => Init/src/ErrorHandling/Infrastructure/Logging/ContentProcessor.php} (61%) rename components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/{LoggingErrorFileStorageAdapter.php => FileWriter.php} (62%) mode change 100644 => 100755 create mode 100755 components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LazySettings.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/CleanUpOldErrorLogsJob.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/Settings.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Logging/FileHandler.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Logging/Settings.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Logging/SettingsInterface.php create mode 100755 components/ILIAS/Init/src/ErrorHandling/Notification/Settings.php rename components/ILIAS/Init/src/ErrorHandling/{Infrastructure/Logging/LoggingErrorLogDirectory.php => Notification/SettingsInterface.php} (65%) mode change 100644 => 100755 rename components/ILIAS/Init/tests/ErrorHandling/{LoggingErrorFileStorageAdapterTest.php => ErrorLoggingFileWriterTest.php} (74%) delete mode 100755 components/ILIAS/Logging/classes/error/class.ilLoggerCronCleanErrorFiles.php delete mode 100755 components/ILIAS/Logging/classes/error/class.ilLoggingErrorSettings.php diff --git a/components/ILIAS/Init/classes/class.ilErrorHandling.php b/components/ILIAS/Init/classes/class.ilErrorHandling.php index 33807ca5b0bd..9e5db386407f 100755 --- a/components/ILIAS/Init/classes/class.ilErrorHandling.php +++ b/components/ILIAS/Init/classes/class.ilErrorHandling.php @@ -106,8 +106,8 @@ protected function initWhoopsHandlers(): void new ErrorHandlers\RecordErrorIncidentHandler( new ErrorHandling\Application\ProductionOnlyErrorIncidentReporting( new ErrorHandling\Application\ReportErrorIncident( - new ErrorLogging\LoggingErrorLogDirectory(), - new ErrorLogging\LoggingErrorFileStorageAdapter(), + new ErrorLogging\LazySettings(), + new ErrorLogging\FileWriter(new ErrorHandling\Logging\FileHandler(), new ErrorLogging\ContentProcessor()), new ErrorHandling\Incident\SessionPrefixedErrorIncidentFactory(), $this->error_incident_registry, self::SENSTIVE_PARAMETER_NAMES @@ -265,9 +265,8 @@ protected function defaultHandler(): HandlerInterface $incident = $this->error_incident_registry->current(); if ($incident !== null) { $language = $DIC->isDependencyAvailable('language') ? $DIC->language() : null; - $message = new ErrorHandling\Notification\ErrorIncidentUserMessage( - ilLoggingErrorSettings::getInstance() - )->format($incident, $language); + $settings = $DIC->isDependencyAvailable('clientIni') ? new ErrorHandling\Notification\Settings($DIC->clientIni()) : null; + $message = new ErrorHandling\Notification\ErrorIncidentUserMessage()->format($incident, $language, $settings); } if ($DIC->isDependencyAvailable('ui') && isset($DIC['tpl']) && $DIC->isDependencyAvailable('ctrl')) { diff --git a/components/ILIAS/Init/service.xml b/components/ILIAS/Init/service.xml index 8274f30dd083..f01bf137e772 100755 --- a/components/ILIAS/Init/service.xml +++ b/components/ILIAS/Init/service.xml @@ -4,5 +4,8 @@ + + + diff --git a/components/ILIAS/Logging/classes/error/class.ilLoggingErrorFileStorage.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/ContentProcessor.php similarity index 61% rename from components/ILIAS/Logging/classes/error/class.ilLoggingErrorFileStorage.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/ContentProcessor.php index 04077df92b4b..8230974466c9 100755 --- a/components/ILIAS/Logging/classes/error/class.ilLoggingErrorFileStorage.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/ContentProcessor.php @@ -18,77 +18,31 @@ declare(strict_types=1); +namespace ILIAS\Init\ErrorHandling\Infrastructure\Logging; + use Whoops\Exception\Formatter; use Whoops\Exception\Inspector; -class ilLoggingErrorFileStorage +class ContentProcessor { - protected const KEY_SPACE = 25; - protected const FILE_FORMAT = '.log'; - - protected Inspector $inspector; - protected string $file_path; - protected string $file_name; - /** @var list */ - private array $exclusion_list = []; - - public function __construct(Inspector $inspector, string $file_path, string $file_name) - { - $this->inspector = $inspector; - $this->file_path = $file_path; - $this->file_name = $file_name; - } + private const int KEY_SPACE = 25; /** - * @param list $exclusion_list + * @param list $sensitive_fields */ - public function withExclusionList(array $exclusion_list): self - { - $clone = clone $this; - $clone->exclusion_list = $exclusion_list; - return $clone; - } - - private function stripNullBytes(string $ret): string - { - return str_replace("\0", '', $ret); - } - - protected function createDir(): void - { - if (!is_dir($this->file_path)) { - ilFileUtils::makeDirParents($this->file_path); - } - } - - protected function content(): string - { - return $this->pageHeader() - . $this->exceptionContent() - . $this->tablesContent(); - } - - public function write(): void - { - $this->createDir(); - - $file_name = $this->file_path . '/' . $this->file_name . self::FILE_FORMAT; - $stream = fopen($file_name, 'wb+'); - fwrite($stream, $this->content()); - fclose($stream); - chmod($file_name, 0755); - } - - protected function pageHeader(): string - { - return ''; + public function collectAndFormatContent( + Inspector $inspector, + array $sensitive_fields + ): string { + return $this->formatExceptionContent($inspector) + . $this->formatTables($this->tablesFromSuperGlobals($sensitive_fields)); } - protected function exceptionContent(): string + private function formatExceptionContent(Inspector $inspector): string { - $message = Formatter::formatExceptionPlain($this->inspector); + $message = Formatter::formatExceptionPlain($inspector); - $exception = $this->inspector->getException(); + $exception = $inspector->getException(); $previous = $exception->getPrevious(); while ($previous) { $message .= "\n\nCaused by\n" . sprintf( @@ -104,10 +58,13 @@ protected function exceptionContent(): string return $message; } - protected function tablesContent(): string + /** + * @param array> $tables + */ + private function formatTables(array $tables): string { $ret = ''; - foreach ($this->tables() as $title => $content) { + foreach ($tables as $title => $content) { $ret .= "\n\n-- $title --\n\n"; if (count($content) > 0) { foreach ($content as $key => $value) { @@ -135,33 +92,40 @@ protected function tablesContent(): string return $this->stripNullBytes($ret); } - protected function tables(): array + /** + * @param list $sensitive_fields + * @return array> + */ + private function tablesFromSuperGlobals(array $sensitive_fields): array { - $post = $_POST; - $server = $_SERVER; + $post = (array) $_POST; + $server = (array) $_SERVER; - $post = $this->hideSensitiveData($post); - $server = $this->hideSensitiveData($server); + $post = $this->hideSensitiveData($post, $sensitive_fields); + $server = $this->hideSensitiveData($server, $sensitive_fields); $server = $this->shortenPHPSessionId($server); return [ - 'GET Data' => $_GET, + 'GET Data' => (array) $_GET, 'POST Data' => $post, - 'Files' => $_FILES, - 'Cookies' => $_COOKIE, - 'Session' => $_SESSION ?? [], + 'Files' => (array) $_FILES, + 'Cookies' => (array) $_COOKIE, + 'Session' => (array) ($_SESSION ?? []), 'Server/Request Data' => $server, - 'Environment Variables' => $_ENV + 'Environment Variables' => (array) $_ENV ]; } /** * @param array $super_global + * @param list $sensitive_fields * @return array */ - private function hideSensitiveData(array $super_global): array - { - foreach ($this->exclusion_list as $parameter) { + private function hideSensitiveData( + array $super_global, + array $sensitive_fields + ): array { + foreach ($sensitive_fields as $parameter) { if (isset($super_global[$parameter])) { $super_global[$parameter] = 'REMOVED FOR SECURITY'; } @@ -174,6 +138,10 @@ private function hideSensitiveData(array $super_global): array return $super_global; } + /** + * @param array $server + * @return array + */ private function shortenPHPSessionId(array $server): array { if (!isset($server['HTTP_COOKIE'])) { @@ -194,4 +162,9 @@ private function shortenPHPSessionId(array $server): array return $server; } + + private function stripNullBytes(string $ret): string + { + return str_replace("\0", '', $ret); + } } diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/FileWriter.php old mode 100644 new mode 100755 similarity index 62% rename from components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php rename to components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/FileWriter.php index bf2ba44f315c..0636ef995e4b --- a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorFileStorageAdapter.php +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/FileWriter.php @@ -20,19 +20,31 @@ namespace ILIAS\Init\ErrorHandling\Infrastructure\Logging; -use ILIAS\Init\ErrorHandling\Application\ErrorLogFileStorage; use Whoops\Exception\Inspector; +use ILIAS\Init\ErrorHandling\Application\ErrorLogFileStorage; +use ILIAS\Init\ErrorHandling\Logging\FileHandler; -final class LoggingErrorFileStorageAdapter implements ErrorLogFileStorage +class FileWriter implements ErrorLogFileStorage { + public function __construct( + private readonly FileHandler $file_handler, + private readonly ContentProcessor $content_processor + ) { + } + + /** + * @param list $sensitive_parameter_names + */ public function write( Inspector $inspector, string $directory, string $file_name, array $sensitive_parameter_names ): void { - $writer = new \ilLoggingErrorFileStorage($inspector, $directory, $file_name); - $writer = $writer->withExclusionList($sensitive_parameter_names); - $writer->write(); + $this->file_handler->createFile( + $directory, + $file_name, + $this->content_processor->collectAndFormatContent($inspector, $sensitive_parameter_names), + ); } } diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LazySettings.php b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LazySettings.php new file mode 100755 index 000000000000..ef5f9d8a0114 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LazySettings.php @@ -0,0 +1,52 @@ +fetchSettings()?->directory() ?? ''; + } + + private function fetchSettings(): ?Settings + { + global $DIC; + + + if ($DIC->offsetExists('ilIliasIniFile')) { + return new Settings($DIC->iliasIni()); + } + if ($DIC->offsetExists('ini')) { + return new Settings($DIC['ini']); + } + return null; + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/CleanUpOldErrorLogsJob.php b/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/CleanUpOldErrorLogsJob.php new file mode 100755 index 000000000000..932190ed0024 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/CleanUpOldErrorLogsJob.php @@ -0,0 +1,142 @@ +lng = $DIC->language(); + $this->lng->loadLanguageModule("logging"); + + $this->settings = new Settings( + new ilSetting('log') + ); + $this->error_log_settings = new ErrorLogSettings($DIC->iliasIni()); + $this->file_handler = new FileHandler(); + } + + public function getId(): string + { + return "log_error_file_cleanup"; + } + + public function getTitle(): string + { + return $this->lng->txt("log_error_file_cleanup_title"); + } + + public function getDescription(): string + { + return $this->lng->txt("log_error_file_cleanup_info"); + } + + public function getDefaultScheduleType(): JobScheduleType + { + return JobScheduleType::IN_DAYS; + } + + public function getDefaultScheduleValue(): int + { + return 10; + } + + public function hasAutoActivation(): bool + { + return false; + } + + public function hasFlexibleSchedule(): bool + { + return true; + } + + public function hasCustomSettings(): bool + { + return true; + } + + public function run(): JobResult + { + $result = new JobResult(); + + $error_log_directory = $this->error_log_settings->directory(); + if (!$this->file_handler->doesDirectoryExist($error_log_directory)) { + $result->setStatus(JobResult::STATUS_OK); + $result->setMessage($this->lng->txt("log_error_path_not_configured_or_wrong")); + return $result; + } + + $deleted_files = $this->file_handler->deleteFilesOlderThan( + $error_log_directory, + $this->settings->deletionCutoffInDays() + ); + + if ($deleted_files > 0) { + $result->setStatus(JobResult::STATUS_OK); + } else { + $result->setStatus(JobResult::STATUS_NO_ACTION); + } + return $result; + } + + public function usesLegacyForms(): bool + { + return false; + } + + public function getCustomConfigurationInput( + UIFactory $ui_factory, + Refinery $factory, + ilLanguage $lng + ): FormInput { + $lng->loadLanguageModule("logging"); + + return $ui_factory->input()->field() + ->numeric($this->lng->txt('frm_clear_older_then'), $this->lng->txt('frm_clear_older_then_info')) + ->withAdditionalTransformation($factory->int()->isGreaterThanOrEqual(1)) + ->withValue($this->settings->deletionCutoffInDays()); + } + + public function saveCustomConfiguration(mixed $form_data): void + { + $this->settings->saveDeletionCutoff((int) $form_data); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/Settings.php b/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/Settings.php new file mode 100755 index 000000000000..98132374fa23 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Logging/CleanUp/Settings.php @@ -0,0 +1,50 @@ +deletion_cutoff_in_days ??= + (int) $this->storage->get('clear_older_then', (string) self::DEFAULT_CUTOFF); + } + + public function saveDeletionCutoff(int $days): void + { + if ($days < 1) { + $days = 1; + } + $this->deletion_cutoff_in_days = $days; + $this->storage->set('clear_older_then', (string) $days); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Logging/FileHandler.php b/components/ILIAS/Init/src/ErrorHandling/Logging/FileHandler.php new file mode 100755 index 000000000000..fc0ee9c1293f --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Logging/FileHandler.php @@ -0,0 +1,97 @@ +createDirectoryIfNecessary($directory); + + $file_name = rtrim($directory, '/') . '/' . $file_name . self::FILE_FORMAT; + $stream = fopen($file_name, 'wb+'); + fwrite($stream, $content); + fclose($stream); + chmod($file_name, 0755); + } + + public function deleteFilesOlderThan( + string $directory, + int $cutoff_in_days + ): int { + $files = $this->readFilesInDirectory($directory); + $delete_date = new DateTimeImmutable(); + $delete_date = $delete_date->sub(new DateInterval('P' . $cutoff_in_days . 'D')); + + $count = 0; + foreach ($files as $file) { + $file_path = rtrim($directory, '/') . '/' . $file; + $file_date = date('Y-m-d', filemtime($file_path)); + + if ($file_date <= $delete_date->format('Y-m-d')) { + $this->deleteFile($file_path); + $count++; + } + } + return $count; + } + + private function createDirectoryIfNecessary(string $directory): void + { + if (!$this->doesDirectoryExist($directory)) { + ilFileUtils::makeDirParents($directory); + } + } + + private function deleteFile(string $file_path): void + { + unlink($file_path); + } + + private function readFilesInDirectory(string $directory): array + { + $ret = []; + + $folder = dir($directory); + while ($file_name = $folder->read()) { + if (filetype(rtrim($directory, '/') . '/' . $file_name) != 'dir') { + $ret[] = $file_name; + } + } + $folder->close(); + + return $ret; + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Logging/Settings.php b/components/ILIAS/Init/src/ErrorHandling/Logging/Settings.php new file mode 100755 index 000000000000..396215c3c66f --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Logging/Settings.php @@ -0,0 +1,36 @@ +ilias_ini->readVariable('log', 'error_path'), '/'); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Logging/SettingsInterface.php b/components/ILIAS/Init/src/ErrorHandling/Logging/SettingsInterface.php new file mode 100755 index 000000000000..722f61759583 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Logging/SettingsInterface.php @@ -0,0 +1,26 @@ +identifier()->value(); + $mail = ''; + if ($notification_settings !== null) { + $mail = $notification_settings->errorRecipient(); + } + if ($language !== null) { $language->loadLanguageModule('logging'); $message = \sprintf($language->txt('log_error_message'), $identifier); - $mail = $this->error_settings->mail(); if ($mail !== '') { $message .= ' ' . \sprintf( $language->txt('log_error_message_send_mail'), @@ -56,7 +58,6 @@ public function format(ErrorIncident $incident, ?\ilLanguage $language = null): $message = 'Sorry, an error occured. A logfile has been created which can be identified via the code "' . $identifier . '"'; - $mail = $this->error_settings->mail(); if ($mail !== '') { $message .= ' ' . 'Please send a mail to ' . $mail . ''; diff --git a/components/ILIAS/Init/src/ErrorHandling/Notification/Settings.php b/components/ILIAS/Init/src/ErrorHandling/Notification/Settings.php new file mode 100755 index 000000000000..9df913ad31e1 --- /dev/null +++ b/components/ILIAS/Init/src/ErrorHandling/Notification/Settings.php @@ -0,0 +1,46 @@ +error_recipient ??= + $this->client_ini->readVariable('log', 'error_recipient'); + } + + public function saveErrorRecipient(string $recipient): void + { + $this->client_ini->addGroup('log'); + $this->client_ini->setVariable('log', 'error_recipient', trim($recipient)); + $this->client_ini->write(); + } +} diff --git a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php b/components/ILIAS/Init/src/ErrorHandling/Notification/SettingsInterface.php old mode 100644 new mode 100755 similarity index 65% rename from components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php rename to components/ILIAS/Init/src/ErrorHandling/Notification/SettingsInterface.php index 30fab2c48bdf..5afe89bba616 --- a/components/ILIAS/Init/src/ErrorHandling/Infrastructure/Logging/LoggingErrorLogDirectory.php +++ b/components/ILIAS/Init/src/ErrorHandling/Notification/SettingsInterface.php @@ -18,14 +18,13 @@ declare(strict_types=1); -namespace ILIAS\Init\ErrorHandling\Infrastructure\Logging; +namespace ILIAS\Init\ErrorHandling\Notification; -use ILIAS\Init\ErrorHandling\Application\ErrorLogDirectory; +use ilIniFile; -final class LoggingErrorLogDirectory implements ErrorLogDirectory +interface SettingsInterface { - public function path(): string - { - return \ilLoggingErrorSettings::getInstance()->folder(); - } + public function errorRecipient(): string; + + public function saveErrorRecipient(string $recipient): void; } diff --git a/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php index 134c7df9a2be..84d65c27e018 100644 --- a/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php +++ b/components/ILIAS/Init/tests/ErrorHandling/ErrorIncidentUserMessageTest.php @@ -21,27 +21,25 @@ use ILIAS\Init\ErrorHandling\Incident\ErrorIncident; use ILIAS\Init\ErrorHandling\Incident\ErrorIncidentId; use ILIAS\Init\ErrorHandling\Notification\ErrorIncidentUserMessage; +use ILIAS\Init\ErrorHandling\Notification\Settings; use PHPUnit\Framework\TestCase; class ErrorIncidentUserMessageTest extends TestCase { - public function testFormatsFallbackMessageWithoutLanguage(): void + public function testFormatsFallbackMessageWithoutLanguageWithMail(): void { - $settings = $this->createMock(ilLoggingErrorSettings::class); - $settings->method('mail')->willReturn('admin@example.org'); + $settings = $this->createMock(Settings::class); + $settings->method('errorRecipient')->willReturn('admin@example.org'); - $message_formatter = new ErrorIncidentUserMessage($settings); - $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), null); + $message_formatter = new ErrorIncidentUserMessage(); + $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), null, $settings); self::assertStringContainsString('abc_12', $message); self::assertStringContainsString('admin@example.org', $message); } - public function testFormatsLocalizedMessageWithLanguage(): void + public function testFormatsLocalizedMessageWithLanguageWithoutMail(): void { - $settings = $this->createMock(ilLoggingErrorSettings::class); - $settings->method('mail')->willReturn(''); - $language = $this->createMock(ilLanguage::class); $language->expects($this->once())->method('loadLanguageModule')->with('logging'); $language->method('txt')->willReturnCallback( @@ -51,8 +49,8 @@ public function testFormatsLocalizedMessageWithLanguage(): void } ); - $message_formatter = new ErrorIncidentUserMessage($settings); - $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), $language); + $message_formatter = new ErrorIncidentUserMessage(); + $message = $message_formatter->format(new ErrorIncident(new ErrorIncidentId('abc_12')), $language, null); self::assertSame('Logged error abc_12', $message); } diff --git a/components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php b/components/ILIAS/Init/tests/ErrorHandling/ErrorLoggingFileWriterTest.php similarity index 74% rename from components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php rename to components/ILIAS/Init/tests/ErrorHandling/ErrorLoggingFileWriterTest.php index 09332741b8c1..0563b5e7bd05 100644 --- a/components/ILIAS/Init/tests/ErrorHandling/LoggingErrorFileStorageAdapterTest.php +++ b/components/ILIAS/Init/tests/ErrorHandling/ErrorLoggingFileWriterTest.php @@ -18,13 +18,15 @@ declare(strict_types=1); -use ILIAS\Init\ErrorHandling\Infrastructure\Logging\LoggingErrorFileStorageAdapter; +use ILIAS\Init\ErrorHandling\Infrastructure\Logging\FileWriter; +use ILIAS\Init\ErrorHandling\Logging\FileHandler; +use ILIAS\Init\ErrorHandling\Infrastructure\Logging\ContentProcessor; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamWrapper; use PHPUnit\Framework\TestCase; use Whoops\Exception\Inspector; -class LoggingErrorFileStorageAdapterTest extends TestCase +class ErrorLoggingFileWriterTest extends TestCase { public function testWritesExceptionDetailsToVirtualLogFile(): void { @@ -37,10 +39,10 @@ public function testWritesExceptionDetailsToVirtualLogFile(): void $log_directory = vfsStream::url('root/errors'); $log_file = vfsStream::url('root/errors/abcde_42.log'); - $inspector = new Inspector(new RuntimeException('adapter test')); + $inspector = new Inspector(new RuntimeException('writer test')); - $adapter = new LoggingErrorFileStorageAdapter(); - $adapter->write( + $writer = new FileWriter(new FileHandler(), new ContentProcessor()); + $writer->write( $inspector, $log_directory, 'abcde_42', @@ -48,7 +50,7 @@ public function testWritesExceptionDetailsToVirtualLogFile(): void ); self::assertFileExists($log_file); - self::assertStringContainsString('adapter test', (string) file_get_contents($log_file)); + self::assertStringContainsString('writer test', (string) file_get_contents($log_file)); } private function skipIfVfsStreamNotAvailable(): void diff --git a/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php b/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php index d9fe44baa31b..78c19f82b739 100644 --- a/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php +++ b/components/ILIAS/Init/tests/ErrorHandling/ReportErrorIncidentTest.php @@ -26,7 +26,9 @@ use ILIAS\Init\ErrorHandling\Incident\ErrorIncidentId; use ILIAS\Init\ErrorHandling\Incident\InMemoryErrorIncidentRegistry; use ILIAS\Init\ErrorHandling\Incident\SessionPrefixedErrorIncidentFactory; -use ILIAS\Init\ErrorHandling\Infrastructure\Logging\LoggingErrorFileStorageAdapter; +use ILIAS\Init\ErrorHandling\Infrastructure\Logging\FileWriter; +use ILIAS\Init\ErrorHandling\Logging\FileHandler; +use ILIAS\Init\ErrorHandling\Infrastructure\Logging\ContentProcessor; use org\bovigo\vfs\vfsStream; use org\bovigo\vfs\vfsStreamWrapper; use PHPUnit\Framework\TestCase; @@ -91,7 +93,7 @@ public function path(): string return $this->path; } }, - new LoggingErrorFileStorageAdapter(), + new FileWriter(new FileHandler(), new ContentProcessor()), $incident_factory, $registry, ['password'] diff --git a/components/ILIAS/Logging/classes/class.ilObjLoggingSettingsGUI.php b/components/ILIAS/Logging/classes/class.ilObjLoggingSettingsGUI.php index fc91f1e100ff..86ae0b302f48 100755 --- a/components/ILIAS/Logging/classes/class.ilObjLoggingSettingsGUI.php +++ b/components/ILIAS/Logging/classes/class.ilObjLoggingSettingsGUI.php @@ -17,9 +17,14 @@ *********************************************************************/ declare(strict_types=1); + use ILIAS\DI\Container; use ILIAS\Refinery\Factory as Refinery; use ILIAS\HTTP\Services as Services; +use ILIAS\Init\ErrorHandling\Logging\Settings as ErrorLogSettings; +use ILIAS\Init\ErrorHandling\Logging\SettingsInterface as ErrorLogSettingsInterface; +use ILIAS\Init\ErrorHandling\Notification\Settings as ErrorNotificationSettings; +use ILIAS\Init\ErrorHandling\Notification\SettingsInterface as ErrorNotificationSettingsInterface; /** * @@ -31,21 +36,17 @@ */ class ilObjLoggingSettingsGUI extends ilObjectGUI { - protected const SECTION_SETTINGS = 'settings'; - protected const SUB_SECTION_MAIN = 'log_general_settings'; - protected const SUB_SECTION_COMPONENTS = 'log_components'; - protected const SUB_SECTION_ERROR = 'log_error_settings'; + protected const string SECTION_SETTINGS = 'settings'; + protected const string SUB_SECTION_MAIN = 'log_general_settings'; + protected const string SUB_SECTION_COMPONENTS = 'log_components'; + protected const string SUB_SECTION_ERROR = 'log_error_settings'; protected ilLoggingDBSettings $log_settings; protected ilLogger $log; - protected ilLoggingErrorSettings $error_settings; + protected ErrorLogSettingsInterface $error_log_settings; + protected ErrorNotificationSettingsInterface $error_notification_settings; protected Refinery $refinery; - /** - * - * @param mixed $a_data - * @param boolean $a_prepare_output - */ public function __construct($a_data, int $a_id, bool $a_call_by_reference, bool $a_prepare_output = true) { global $DIC; @@ -56,11 +57,13 @@ public function __construct($a_data, int $a_id, bool $a_call_by_reference, bool $this->lng = $DIC->language(); $this->initSettings(); - $this->initErrorSettings(); $this->lng->loadLanguageModule('logging'); $this->lng->loadLanguageModule('log'); $this->log = ilLoggerFactory::getLogger('log'); + $this->error_log_settings = new ErrorLogSettings($DIC->iliasIni()); + $this->error_notification_settings = new ErrorNotificationSettings($DIC->clientIni()); + $this->refinery = $DIC->refinery(); } @@ -312,8 +315,7 @@ protected function updateErrorSettings(): void $this->checkPermission('write'); $form = $this->initFormErrorSettings(); if ($form->checkInput()) { - $this->getErrorSettings()->setMail($form->getInput('error_mail')); - $this->getErrorSettings()->update(); + $this->error_notification_settings->saveErrorRecipient($form->getInput('error_mail')); $this->tpl->setOnScreenMessage('success', $this->lng->txt('error_settings_saved'), true); $this->ctrl->redirect($this, 'errorSettings'); @@ -334,22 +336,12 @@ protected function initFormErrorSettings(): ilPropertyFormGUI } $folder = new ilNonEditableValueGUI($this->lng->txt('log_error_folder'), 'error_folder'); - $folder->setValue($this->getErrorSettings()->folder()); + $folder->setValue($this->error_log_settings->directory()); $form->addItem($folder); $mail = new ilTextInputGUI($this->lng->txt('log_error_mail'), 'error_mail'); - $mail->setValue($this->getErrorSettings()->mail()); + $mail->setValue($this->error_notification_settings->errorRecipient()); $form->addItem($mail); return $form; } - - protected function initErrorSettings(): void - { - $this->error_settings = ilLoggingErrorSettings::getInstance(); - } - - protected function getErrorSettings(): ilLoggingErrorSettings - { - return $this->error_settings; - } } diff --git a/components/ILIAS/Logging/classes/error/class.ilLoggerCronCleanErrorFiles.php b/components/ILIAS/Logging/classes/error/class.ilLoggerCronCleanErrorFiles.php deleted file mode 100755 index 748470e98756..000000000000 --- a/components/ILIAS/Logging/classes/error/class.ilLoggerCronCleanErrorFiles.php +++ /dev/null @@ -1,163 +0,0 @@ -lng = $DIC->language(); - $this->lng->loadLanguageModule("logging"); - $this->settings = new ilSetting('log'); - $this->error_settings = ilLoggingErrorSettings::getInstance(); - } - - public function getId(): string - { - return "log_error_file_cleanup"; - } - - public function getTitle(): string - { - return $this->lng->txt("log_error_file_cleanup_title"); - } - - public function getDescription(): string - { - return $this->lng->txt("log_error_file_cleanup_info"); - } - - public function getDefaultScheduleType(): JobScheduleType - { - return JobScheduleType::IN_DAYS; - } - - public function getDefaultScheduleValue(): int - { - return 10; - } - - public function hasAutoActivation(): bool - { - return false; - } - - public function hasFlexibleSchedule(): bool - { - return true; - } - - public function hasCustomSettings(): bool - { - return true; - } - - public function run(): JobResult - { - $result = new JobResult(); - $folder = $this->error_settings->folder(); - if (!is_dir($folder)) { - $result->setStatus(JobResult::STATUS_OK); - $result->setMessage($this->lng->txt("log_error_path_not_configured_or_wrong")); - return $result; - } - - $offset = $this->settings->get('clear_older_then', ''); - if ($offset) { - $offset = (int) $offset; - } else { - $offset = self::DEFAULT_VALUE_OLDER_THAN; - } - - $files = $this->readLogDir($folder); - $delete_date = new ilDateTime(date("Y-m-d"), IL_CAL_DATE); - $delete_date->increment(ilDateTime::DAY, (-1 * $offset)); - - foreach ($files as $file) { - $file_date = date("Y-m-d", filemtime($this->error_settings->folder() . "/" . $file)); - - if ($file_date <= $delete_date->get(IL_CAL_DATE)) { - $this->deleteFile($this->error_settings->folder() . "/" . $file); - } - } - - $result->setStatus(JobResult::STATUS_OK); - return $result; - } - - protected function readLogDir(string $path): array - { - $ret = []; - - $folder = dir($path); - while ($file_name = $folder->read()) { - if (filetype($path . "/" . $file_name) != "dir") { - $ret[] = $file_name; - } - } - $folder->close(); - - return $ret; - } - - protected function deleteFile(string $path): void - { - unlink($path); - } - - public function addCustomSettingsToForm(ilPropertyFormGUI $a_form): void - { - $offset = $this->settings->get('clear_older_then', ''); - if (!$offset) { - $offset = (string) self::DEFAULT_VALUE_OLDER_THAN; - } - - $clear_older_then = new ilNumberInputGUI($this->lng->txt('frm_clear_older_then'), 'clear_older_then'); - $clear_older_then->allowDecimals(false); - $clear_older_then->setMinValue(1, true); - $clear_older_then->setValue($offset); - $clear_older_then->setInfo($this->lng->txt('frm_clear_older_then_info')); - - $a_form->addItem($clear_older_then); - } - - public function saveCustomSettings(ilPropertyFormGUI $a_form): bool - { - $threshold = $a_form->getInput('clear_older_then'); - if ((string) $threshold === '') { - $this->settings->delete('clear_older_then'); - } else { - $this->settings->set('clear_older_then', (string) ((int) $a_form->getInput('clear_older_then'))); - } - - return true; - } -} diff --git a/components/ILIAS/Logging/classes/error/class.ilLoggingErrorSettings.php b/components/ILIAS/Logging/classes/error/class.ilLoggingErrorSettings.php deleted file mode 100755 index ecebb7c466e8..000000000000 --- a/components/ILIAS/Logging/classes/error/class.ilLoggingErrorSettings.php +++ /dev/null @@ -1,95 +0,0 @@ - - */ -class ilLoggingErrorSettings -{ - protected string $folder = ''; - protected string $mail = ''; - protected ?ilIniFile $ilias_ini = null; - protected ?ilIniFile $gClientIniFile = null; - - protected function __construct() - { - global $DIC; - - if ($DIC->offsetExists('ilIliasIniFile')) { - $this->ilias_ini = $DIC->iliasIni(); - } elseif ($DIC->offsetExists('ini')) { - $this->ilias_ini = $DIC['ini']; - } - if ($DIC->offsetExists('ilClientIniFile')) { - $this->gClientIniFile = $DIC->clientIni(); - } - $this->read(); - } - - public static function getInstance(): ilLoggingErrorSettings - { - return new ilLoggingErrorSettings(); - } - - protected function setFolder(string $folder): void - { - $this->folder = $folder; - } - - public function setMail(string $mail): void - { - $this->mail = $mail; - } - - public function folder(): string - { - return $this->folder; - } - - public function mail(): string - { - return $this->mail; - } - - /** - * reads the values from ilias.ini.php - */ - protected function read(): void - { - if ($this->ilias_ini instanceof ilIniFile) { - $this->setFolder((string) $this->ilias_ini->readVariable("log", "error_path")); - } - if ($this->gClientIniFile instanceof \ilIniFile) { - $this->setMail((string) $this->gClientIniFile->readVariable("log", "error_recipient")); - } - } - - /** - * writes mail recipient into client.ini.php - */ - public function update(): void - { - if ($this->gClientIniFile instanceof \ilIniFile) { - $this->gClientIniFile->addGroup("log"); - $this->gClientIniFile->setVariable("log", "error_recipient", trim($this->mail())); - $this->gClientIniFile->write(); - } - } -} diff --git a/components/ILIAS/Logging/service.xml b/components/ILIAS/Logging/service.xml index 13cdbbe73f12..31e109a29e12 100755 --- a/components/ILIAS/Logging/service.xml +++ b/components/ILIAS/Logging/service.xml @@ -9,8 +9,5 @@ adm - - -