Skip to content

Commit

Permalink
Avoid making db connection when logging about database connection
Browse files Browse the repository at this point in the history
For various reasons, sometimes a database would not be possible. Our file
 logger before this commit has a function call that relies on a db connection
 that is ironic and creates some kind of chicken and egg scenario. This commit,
resolves that.

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
  • Loading branch information
Fenn-CS committed Apr 14, 2023
1 parent f3f988c commit 130b827
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
5 changes: 5 additions & 0 deletions index.php
Expand Up @@ -30,6 +30,7 @@
*/
require_once __DIR__ . '/lib/versioncheck.php';


try {
require_once __DIR__ . '/lib/base.php';

Expand All @@ -38,6 +39,7 @@
\OC::$server->getPsrLogger()->error($ex->getMessage(), [
'exception' => $ex,
]);
\OC::getLogFile()->write('core', $ex->getMessage(), 3);

//show the user a detailed error page
OC_Template::printExceptionErrorPage($ex, 503);
Expand All @@ -60,6 +62,7 @@
OC_Template::printExceptionErrorPage($ex, 500);
}
} catch (\OC\User\LoginException $ex) {
\OC::getLogFile()->write('core', $ex->getMessage(), 3);
$request = \OC::$server->getRequest();
/**
* Routes with the @CORS annotation and other API endpoints should
Expand All @@ -74,13 +77,15 @@
}
OC_Template::printErrorPage($ex->getMessage(), $ex->getMessage(), 401);
} catch (Exception $ex) {
\OC::getLogFile()->write('core', $ex->getMessage(), 3);
\OC::$server->getPsrLogger()->error($ex->getMessage(), [
'exception' => $ex,
]);

//show the user a detailed error page
OC_Template::printExceptionErrorPage($ex, 500);
} catch (Error $ex) {
\OC::getLogFile()->write('core', $ex->getMessage(), 3);
try {
\OC::$server->getPsrLogger()->error($ex->getMessage(), [
'exception' => $ex,
Expand Down
16 changes: 16 additions & 0 deletions lib/base.php
Expand Up @@ -1160,6 +1160,22 @@ protected static function handleAuthHeaders(): void {
}
}
}

/*
* Sometimes the database service might not be available, see issue #37424
* hence getting configuration via the database would not be possible.
* This method would return the logfile path read directly from the config
* file. This is an anti-pattern and only done because there's no other way
* to gracefully handle database connection errors. We also don't want to expose the full
* self::$config object as it might contain important secrets.
*/
public static function getLogFilePath(): string {
return self::$config->getValue('logfile', self::$SERVERROOT . '/data' . '/nextcloud.log');
}

public static function getLogFile(): \OC\Log\File {
return new \OC\Log\File(self::getLogFilePath(), '', \OC::$server->get(\OC\SystemConfig::class));
}
}

OC::init();
10 changes: 9 additions & 1 deletion lib/private/Log/File.php
Expand Up @@ -80,7 +80,15 @@ public function __construct(string $path, string $fallbackPath, SystemConfig $co
* @param int $level
*/
public function write(string $app, $message, int $level) {
$entry = $this->logDetailsAsJSON($app, $message, $level);
try {
$entry = $this->logDetailsAsJSON($app, $message, $level);
} catch (\Doctrine\DBAL\Exception $e) {
// The existing logger system deeply depends on the database to write any logs
// This means if there is no connection to the database nothing can be logged
// Including the database connection error
// We catch the database connection here and avoid the part (\OC\Log\LogDetails) that relies on the the database.
$entry = json_encode(['app' => $app, 'message' => $message, 'level' => 1]);
}
$handle = @fopen($this->logFile, 'a');
if ($this->logFileMode > 0 && is_file($this->logFile) && (fileperms($this->logFile) & 0777) != $this->logFileMode) {
@chmod($this->logFile, $this->logFileMode);
Expand Down

0 comments on commit 130b827

Please sign in to comment.