From 5df4f429e0fb2c55bc210e6dec344fd7fc5059f1 Mon Sep 17 00:00:00 2001 From: Charles Cote Date: Wed, 30 Sep 2026 18:00:53 +0200 Subject: [PATCH 1/2] Fix repeated web server error log failures --- tests/Unit/WebServerTest.php | 93 ++++++++++++++++++++++++++++++++++++ tests/WebServer.php | 21 +++++++- 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/WebServerTest.php diff --git a/tests/Unit/WebServerTest.php b/tests/Unit/WebServerTest.php new file mode 100644 index 0000000000..0b8d9e4057 --- /dev/null +++ b/tests/Unit/WebServerTest.php @@ -0,0 +1,93 @@ +temporaryDirectory) { + @unlink($this->temporaryDirectory . '/index.php'); + @unlink($this->temporaryDirectory . '/' . WebServer::ERROR_LOG_NAME); + @rmdir($this->temporaryDirectory); + } + + parent::ddTearDown(); + } + + public function testCheckErrorsConsumesLogEntriesOnce() + { + $server = $this->createWebServer(); + $errorLog = $this->temporaryDirectory . '/' . WebServer::ERROR_LOG_NAME; + + file_put_contents($errorLog, "[ddtrace] [error] first error\n"); + + $this->assertSame('[ddtrace] [error] first error', $server->checkErrors()); + $this->assertNull($server->checkErrors()); + + file_put_contents($errorLog, "[ddtrace] [warn] second error\n", FILE_APPEND); + + $this->assertSame('[ddtrace] [warn] second error', $server->checkErrors()); + $this->assertNull($server->checkErrors()); + } + + public function testCheckErrorsHandlesTruncatedLog() + { + $server = $this->createWebServer(); + $errorLog = $this->temporaryDirectory . '/' . WebServer::ERROR_LOG_NAME; + + file_put_contents($errorLog, "[ddtrace] [error] a deliberately long error\n"); + $server->checkErrors(); + + file_put_contents($errorLog, "[ddtrace] [error] new\n"); + + $this->assertSame('[ddtrace] [error] new', $server->checkErrors()); + $this->assertNull($server->checkErrors()); + } + + private function createWebServer() + { + $temporaryFile = tempnam(sys_get_temp_dir(), 'ddtrace-webserver-test-'); + @unlink($temporaryFile); + mkdir($temporaryFile); + $this->temporaryDirectory = $temporaryFile; + + $indexFile = $this->temporaryDirectory . '/index.php'; + touch($indexFile); + + $server = new WebServer($indexFile); + $sapi = new WebServerTestSapi(); + $property = new \ReflectionProperty($server, 'sapi'); + $property->setAccessible(true); + $property->setValue($server, $sapi); + + return $server; + } +} + +final class WebServerTestSapi implements Sapi +{ + public function start() + { + } + + public function stop() + { + } + + public function isFastCgi() + { + return false; + } + + public function checkErrors() + { + return null; + } +} diff --git a/tests/WebServer.php b/tests/WebServer.php index 5652ad4853..9cbe07c555 100644 --- a/tests/WebServer.php +++ b/tests/WebServer.php @@ -358,7 +358,26 @@ public function mergeInis($inis) */ public function checkErrors() { - $diff = @file_get_contents($this->defaultInis['error_log'], false, null, $this->errorLogSize); + $errorLog = $this->defaultInis['error_log']; + + // Only inspect log entries written since the previous check. Without + // advancing the offset, one transient error is reported again by every + // retry and every subsequent test that shares this web server. + clearstatcache(true, $errorLog); + $currentSize = (int) @filesize($errorLog); + if ($currentSize < $this->errorLogSize) { + // The log may have been truncated or rotated while the server was + // running. Start reading from the beginning of the replacement. + $this->errorLogSize = 0; + } + + $diff = @file_get_contents($errorLog, false, null, $this->errorLogSize); + if ($diff === false) { + $diff = ""; + } else { + $this->errorLogSize += strlen($diff); + } + $out = ""; foreach (explode("\n", $diff) as $line) { // Ignore sidecar retry errors for known-invalid test hostnames — these are From f87daee4eaef6b7114e0a173fe1d0732ae59b04c Mon Sep 17 00:00:00 2001 From: Charles Cote Date: Fri, 2 Oct 2026 14:20:12 +0200 Subject: [PATCH 2/2] Detect web server error log replacement via inode, not size alone A size-only comparison misses a rotated/replaced error log whose replacement has already grown to at least the previous offset, causing checkErrors() to skip entries at the beginning of the new file. Track the log's inode in addition to its size and reset the cursor whenever either changes. --- tests/Unit/WebServerTest.php | 22 ++++++++++++++++++++++ tests/WebServer.php | 12 +++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/Unit/WebServerTest.php b/tests/Unit/WebServerTest.php index 0b8d9e4057..af7e57900e 100644 --- a/tests/Unit/WebServerTest.php +++ b/tests/Unit/WebServerTest.php @@ -51,6 +51,28 @@ public function testCheckErrorsHandlesTruncatedLog() $this->assertNull($server->checkErrors()); } + public function testCheckErrorsHandlesReplacedLogGrownPastPreviousOffset() + { + $server = $this->createWebServer(); + $errorLog = $this->temporaryDirectory . '/' . WebServer::ERROR_LOG_NAME; + + // Long enough that a naive size-only comparison would not detect the + // replacement once the new file grows to at least this offset. + $padding = str_repeat('x', 4096); + file_put_contents($errorLog, "[ddtrace] [error] first error $padding\n"); + $server->checkErrors(); + + // Simulate rotation: create a new file elsewhere and rename it over the + // old path, guaranteeing a new inode, already containing more bytes than + // the previous offset. + $replacement = $errorLog . '.1'; + file_put_contents($replacement, "[ddtrace] [error] early marker in replacement\n$padding\n"); + rename($replacement, $errorLog); + + $this->assertSame('[ddtrace] [error] early marker in replacement', $server->checkErrors()); + $this->assertNull($server->checkErrors()); + } + private function createWebServer() { $temporaryFile = tempnam(sys_get_temp_dir(), 'ddtrace-webserver-test-'); diff --git a/tests/WebServer.php b/tests/WebServer.php index 9cbe07c555..3a83f0e99e 100644 --- a/tests/WebServer.php +++ b/tests/WebServer.php @@ -93,6 +93,7 @@ final class WebServer ]; private $errorLogSize = 0; + private $errorLogInode = 0; /** * Persisted apache instance for the lifetime of the testsuite - we use reload instead of restart to apply changes. @@ -184,6 +185,7 @@ public function start() } $this->errorLogSize = (int)@filesize($this->defaultInis['error_log']); + $this->errorLogInode = (int)@fileinode($this->defaultInis['error_log']); if ($this->roadrunnerVersion) { $this->sapi = new RoadrunnerServer( @@ -365,11 +367,15 @@ public function checkErrors() // retry and every subsequent test that shares this web server. clearstatcache(true, $errorLog); $currentSize = (int) @filesize($errorLog); - if ($currentSize < $this->errorLogSize) { - // The log may have been truncated or rotated while the server was - // running. Start reading from the beginning of the replacement. + $currentInode = (int) @fileinode($errorLog); + if ($currentSize < $this->errorLogSize || $currentInode !== $this->errorLogInode) { + // The log may have been truncated, rotated, or replaced while the + // server was running. Comparing the inode detects a replacement that + // has already grown to at least the previous offset, which a size-only + // check would miss. Start reading from the beginning of the new file. $this->errorLogSize = 0; } + $this->errorLogInode = $currentInode; $diff = @file_get_contents($errorLog, false, null, $this->errorLogSize); if ($diff === false) {