Skip to content

Init/ErrorHandling: absorb error log handling from Logging#11809

Open
schmitz-ilias wants to merge 13 commits into
ILIAS-eLearning:trunkfrom
leifos-gmbh:12_error_logging_to_init
Open

Init/ErrorHandling: absorb error log handling from Logging#11809
schmitz-ilias wants to merge 13 commits into
ILIAS-eLearning:trunkfrom
leifos-gmbh:12_error_logging_to_init

Conversation

@schmitz-ilias

Copy link
Copy Markdown
Contributor

With this PR I propose to move all handling of error logs from Logging (specifically classes/error) to Init\ErrorHandling. This includes infrastructure for file handling and formatting of the error logs, a bit of configuration, and the 'Delete old or orphaned error log files' cron job.

As briefly discussed with @mjansenDatabay, this PR builds on the refactoring of Init\ErrorHandling from #11658. All of my changes are on top in a5f30a6.

In the discussion under #11658 @thibsy has voiced the opinion that things should rather move in the opposite direction, and Logging should take over the whole error log infrastructure (and encapsulate it better). In my opinion, the approach here represents a better division of responsibilities between components:

  • The code currently in classes/error is almost fully independent from the rest of Logging. In particular, handling of the (non-error) log file is left to Monolog, while handling of the error logs is done 'by hand' in ilLoggingErrorFileStorage.
  • Relatedly, the code currently in classes/error does depend on Whoops. Moving these classes to Init\ErrorHandling then allows us to keep Whoops fully encapsulated in Init\ErrorHandling (and Monolog in Logging).
  • At least currently, with the component revision not yet completed, Init\ErrorHandling is very particular about its dependencies, and needs to be able to react properly to e.g. a service not being intialized yet. The code being centralized makes it easier to implement this properly.

Besides just moving the code, I also cleaned it up and restructured it a bit (but there is of course still much room for improvement). What I have left in Logging (so far) is the GUI for 'Administration > Logging > Error logging settings'. Splitting up ilObjLoggingSettingsGUI would have been a bit too much work for a proposal. As a consequence, I had to instantiate a few of the new Settings classes by hand, see here. In the (hopefully near) future, when the ini files are accessible during bootstrapping, this can be cleaned up a bit: Init can offer these Settings via the component mechanism, and ilObjLoggingSettingsGUI can grab them from the DIC (via Init\AddModernComponents).

What do you think?

Cheers, Tim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants