From 4388e3de63d0d57268f2417a190c7c2e070d74b1 Mon Sep 17 00:00:00 2001 From: Albert Chen Date: Mon, 2 Nov 2026 11:47:29 +0800 Subject: [PATCH 1/4] fix: treat failed Redis PING replies as unreachable phpredis returns false instead of throwing when Redis answers PING with an ERR reply, such as "max number of clients reached". The Redis, queue, and session reachability diagnostics only treated exceptions as failures, so they reported a rejecting server as healthy. Each probe now checks for a false PING result and reports the affected connection as unreachable. --- .../QueueConnectionIsReachable.php | 5 ++++- .../RedisConnectionsAreReachable.php | 5 ++++- .../Diagnostics/SessionDriverIsReachable.php | 7 ++++++- .../QueueConnectionIsReachableTest.php | 16 ++++++++++++++++ .../RedisConnectionsAreReachableTest.php | 19 +++++++++++++++++++ .../SessionDriverIsReachableTest.php | 12 ++++++++++++ 6 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/doctor/src/Diagnostics/QueueConnectionIsReachable.php b/src/doctor/src/Diagnostics/QueueConnectionIsReachable.php index b1a5b816e..dd58b48b0 100644 --- a/src/doctor/src/Diagnostics/QueueConnectionIsReachable.php +++ b/src/doctor/src/Diagnostics/QueueConnectionIsReachable.php @@ -135,7 +135,10 @@ private function probeRedisQueue(array $configuration): void { $connection = $configuration['connection'] ?? 'default'; - Redis::connection($connection)->ping(); + // phpredis returns false instead of throwing for ERR replies such as "max number of clients reached". + if (Redis::connection($connection)->ping() === false) { + throw new RuntimeException("Redis connection [{$connection}] returned an error for PING."); + } } /** diff --git a/src/doctor/src/Diagnostics/RedisConnectionsAreReachable.php b/src/doctor/src/Diagnostics/RedisConnectionsAreReachable.php index 7cb232eee..8de1d3847 100644 --- a/src/doctor/src/Diagnostics/RedisConnectionsAreReachable.php +++ b/src/doctor/src/Diagnostics/RedisConnectionsAreReachable.php @@ -52,7 +52,10 @@ public function check(): DiagnosticResult foreach ($connections as $connection) { try { - Redis::connection($connection)->ping(); + // phpredis returns false instead of throwing for ERR replies such as "max number of clients reached". + if (Redis::connection($connection)->ping() === false) { + $failures[$connection] = 'The Redis server returned an error for PING.'; + } } catch (CanceledException $exception) { throw $exception; } catch (Throwable $e) { diff --git a/src/doctor/src/Diagnostics/SessionDriverIsReachable.php b/src/doctor/src/Diagnostics/SessionDriverIsReachable.php index fd1a774cf..6bd9b467f 100644 --- a/src/doctor/src/Diagnostics/SessionDriverIsReachable.php +++ b/src/doctor/src/Diagnostics/SessionDriverIsReachable.php @@ -114,7 +114,12 @@ private function probeDatabaseSessions(): void */ private function probeRedisSessions(): void { - Redis::connection(Configured::string('session.connection', SessionManager::DEFAULT_REDIS_CONNECTION))->ping(); + $connection = Configured::string('session.connection', SessionManager::DEFAULT_REDIS_CONNECTION); + + // phpredis returns false instead of throwing for ERR replies such as "max number of clients reached". + if (Redis::connection($connection)->ping() === false) { + throw new RuntimeException("Redis connection [{$connection}] returned an error for PING."); + } } /** diff --git a/tests/Doctor/Feature/Diagnostics/QueueConnectionIsReachableTest.php b/tests/Doctor/Feature/Diagnostics/QueueConnectionIsReachableTest.php index f37067931..da54578d8 100644 --- a/tests/Doctor/Feature/Diagnostics/QueueConnectionIsReachableTest.php +++ b/tests/Doctor/Feature/Diagnostics/QueueConnectionIsReachableTest.php @@ -8,7 +8,9 @@ use Hypervel\Contracts\Queue\Queue as QueueContract; use Hypervel\Doctor\Diagnostics\QueueConnectionIsReachable; use Hypervel\Doctor\Results\Link; +use Hypervel\Redis\RedisProxy; use Hypervel\Support\Facades\Queue; +use Hypervel\Support\Facades\Redis; use Hypervel\Support\Facades\Schema; use Hypervel\Tests\Doctor\TestCase; use Hypervel\Tests\Queue\Fixtures\IntegerQueueName; @@ -104,6 +106,20 @@ public static function failoverConnections(): array ]; } + public function testReportsRedisQueuesWhenTheServerRejectsThePing(): void + { + config([ + 'queue.default' => 'redis', + 'queue.connections.redis' => ['driver' => 'redis', 'connection' => 'queue'], + ]); + Redis::shouldReceive('connection')->with('queue')->andReturn(m::mock(RedisProxy::class, ['ping' => false])); + + $result = (new QueueConnectionIsReachable)->check(); + + $this->assertSame('fail', $result->status->value); + $this->assertStringContainsString('Redis connection [queue] returned an error for PING.', $result->details); + } + public function testBuiltInNullConnectionNeedsNoConfigurationRecord(): void { config(['queue.default' => 'null', 'queue.connections' => []]); diff --git a/tests/Doctor/Feature/Diagnostics/RedisConnectionsAreReachableTest.php b/tests/Doctor/Feature/Diagnostics/RedisConnectionsAreReachableTest.php index 1a32e709f..b7a5e1394 100644 --- a/tests/Doctor/Feature/Diagnostics/RedisConnectionsAreReachableTest.php +++ b/tests/Doctor/Feature/Diagnostics/RedisConnectionsAreReachableTest.php @@ -7,8 +7,10 @@ use Hypervel\Contracts\Foundation\Application; use Hypervel\Doctor\Diagnostics\RedisConnectionsAreReachable; use Hypervel\Doctor\Results\Link; +use Hypervel\Redis\RedisProxy; use Hypervel\Support\Facades\Redis; use Hypervel\Tests\Doctor\TestCase; +use Mockery as m; use RuntimeException; class RedisConnectionsAreReachableTest extends TestCase @@ -135,6 +137,23 @@ public function testSkipsWhenRedisIsOnlyScaffoldedAndUnused(): void $this->assertSame('skip', $result->status->value); } + public function testReportsConnectionsWhoseServerRejectsThePing(): void + { + config([ + 'cache.default' => 'redis', + 'cache.stores.redis' => [ + 'driver' => 'redis', + 'connection' => 'default', + ], + ]); + Redis::shouldReceive('connection')->with('default')->andReturn(m::mock(RedisProxy::class, ['ping' => false])); + + $result = (new RedisConnectionsAreReachable)->check(); + + $this->assertSame('fail', $result->status->value); + $this->assertStringContainsString('The Redis server returned an error for PING.', $result->details); + } + public function testLinksToRedisConfigurationWhenAnActiveConnectionCannotBeReached(): void { config([ diff --git a/tests/Doctor/Feature/Diagnostics/SessionDriverIsReachableTest.php b/tests/Doctor/Feature/Diagnostics/SessionDriverIsReachableTest.php index 9d20f330b..86d83079a 100644 --- a/tests/Doctor/Feature/Diagnostics/SessionDriverIsReachableTest.php +++ b/tests/Doctor/Feature/Diagnostics/SessionDriverIsReachableTest.php @@ -7,6 +7,7 @@ use Hypervel\Database\Schema\Builder; use Hypervel\Doctor\Diagnostics\SessionDriverIsReachable; use Hypervel\Doctor\Results\Link; +use Hypervel\Redis\RedisProxy; use Hypervel\Support\Facades\File; use Hypervel\Support\Facades\Redis; use Hypervel\Support\Facades\Schema; @@ -116,6 +117,17 @@ public static function redisConnections(): array return [[null, 'session'], ['', 'session'], ['custom', 'custom']]; } + public function testReportsRedisSessionsWhenTheServerRejectsThePing(): void + { + config(['session.driver' => 'redis', 'session.connection' => 'session']); + Redis::shouldReceive('connection')->with('session')->andReturn(m::mock(RedisProxy::class, ['ping' => false])); + + $result = (new SessionDriverIsReachable)->check(); + + $this->assertSame('fail', $result->status->value); + $this->assertSame('Redis connection [session] returned an error for PING.', $result->details); + } + public function testDatabaseSessionsKeepTheDefaultSchemaConnection(): void { config(['session.driver' => 'database', 'session.connection' => null, 'session.table' => 'custom_sessions']); From 2923ad8bf9238548d689a6f5d8c79ad4dab70143 Mon Sep 17 00:00:00 2001 From: Albert Chen Date: Mon, 2 Nov 2026 11:47:40 +0800 Subject: [PATCH 2/4] fix: read composer.json on every Doctor check ComposerJson memoized the decoded manifest, but the container keeps one instance for the worker lifetime. Repeated Doctor runs in a long-lived process, such as a scheduled doctor command, kept checking the PHP and extension requirements from the first read after composer.json changed. ComposerJson now decodes the file on each call instead of caching it. --- src/doctor/src/Support/ComposerJson.php | 22 +++++++------------ ...ersionSatisfiesComposerRequirementTest.php | 12 ++++++++++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/doctor/src/Support/ComposerJson.php b/src/doctor/src/Support/ComposerJson.php index 8f5b12e96..3a8af6920 100644 --- a/src/doctor/src/Support/ComposerJson.php +++ b/src/doctor/src/Support/ComposerJson.php @@ -9,13 +9,6 @@ class ComposerJson { - /** - * The decoded composer.json payload. - * - * @var null|array - */ - private ?array $contents = null; - /** * Determine whether the application has a composer.json file. */ @@ -41,9 +34,11 @@ public function phpConstraint(): ?string */ public function requiredExtensions(): array { + $contents = $this->contents() ?? []; + return $this->extensionsFrom([ - ...$this->packageNames(Arr::get($this->contents() ?? [], 'require', [])), - ...$this->packageNames(Arr::get($this->contents() ?? [], 'require-dev', [])), + ...$this->packageNames(Arr::get($contents, 'require', [])), + ...$this->packageNames(Arr::get($contents, 'require-dev', [])), ]); } @@ -62,14 +57,13 @@ public function suggestedExtensions(): array /** * Get the decoded composer.json contents. * + * Read on every call because this class is auto-singletoned for the worker + * lifetime, so a memoized copy would go stale between Doctor runs. + * * @return null|array */ public function contents(): ?array { - if ($this->contents !== null) { - return $this->contents; - } - if (! $this->exists()) { return null; } @@ -81,7 +75,7 @@ public function contents(): ?array } /** @var array $decoded */ - return $this->contents = $decoded; + return $decoded; } /** diff --git a/tests/Doctor/Feature/Diagnostics/PhpVersionSatisfiesComposerRequirementTest.php b/tests/Doctor/Feature/Diagnostics/PhpVersionSatisfiesComposerRequirementTest.php index 9049414b6..96b25183c 100644 --- a/tests/Doctor/Feature/Diagnostics/PhpVersionSatisfiesComposerRequirementTest.php +++ b/tests/Doctor/Feature/Diagnostics/PhpVersionSatisfiesComposerRequirementTest.php @@ -104,6 +104,18 @@ public function testRechecksInstalledDependencyRequirementsAfterMetadataChanges( $this->assertSame('pass', $diagnostic->check()->status->value); } + public function testRechecksRootRequirementsAfterComposerJsonChanges(): void + { + file_put_contents($this->basePath . '/composer.json', '{"require":{"php":">=8.4"}}'); + $diagnostic = $this->app->make(PhpVersionSatisfiesComposerRequirement::class); + + $this->assertSame('pass', $diagnostic->check()->status->value); + + file_put_contents($this->basePath . '/composer.json', '{"require":{"php":">=99"}}'); + + $this->assertSame('fail', $diagnostic->check()->status->value); + } + #[DataProvider('phpVersions')] public function testChecksDistributionAndPrereleaseVersions(string $version, string $constraint, string $status): void { From 5ce8544182d3f76a84162547a549b6aedc966c3f Mon Sep 17 00:00:00 2001 From: Albert Chen Date: Mon, 2 Nov 2026 11:47:40 +0800 Subject: [PATCH 3/4] fix: report blank required default drivers RequiredConfigurationValuesAreSet substituted hardcoded drivers when the default broadcasting, filesystem, mail, or queue setting was blank or null. The managers read these keys with typed getters and fail at runtime instead, so a blank value such as MAIL_MAILER= passed the diagnostic while the application could not resolve its default driver. Blank or null defaults are now reported as missing values, and their driver-specific checks are skipped. A blank logging.default no longer assumes the stack channel, since LogManager treats a missing default channel as optional and uses its emergency logger. --- .../RequiredConfigurationValuesAreSet.php | 67 +++++++++++++++++-- .../RequiredConfigurationValuesAreSetTest.php | 35 ++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/src/doctor/src/Diagnostics/RequiredConfigurationValuesAreSet.php b/src/doctor/src/Diagnostics/RequiredConfigurationValuesAreSet.php index 6ddb298bf..f0c09849f 100644 --- a/src/doctor/src/Diagnostics/RequiredConfigurationValuesAreSet.php +++ b/src/doctor/src/Diagnostics/RequiredConfigurationValuesAreSet.php @@ -60,12 +60,37 @@ private function missingValues(): array $required = $this->requiredValues(); return [ + ...$this->missingDefaultValues(), ...array_intersect_key($required, array_flip(Configured::missing(array_keys($required)))), ...$this->missingMercureValues(), ...$this->missingMailValues(), ]; } + /** + * Get default driver keys without a usable value, mapped to the feature that needs them. + * + * The managers read these keys with typed getters, so a blank or null + * value fails at runtime instead of falling back to a default driver. + * + * @return array + */ + private function missingDefaultValues(): array + { + $defaults = [ + 'broadcasting.default' => 'default broadcaster', + 'filesystems.default' => 'default filesystem disk', + 'mail.default' => 'default mailer', + 'queue.default' => 'default queue connection', + ]; + + return array_filter( + $defaults, + static fn (string $key): bool => Configured::string($key) === null, + ARRAY_FILTER_USE_KEY, + ); + } + /** * Get configuration keys required by the active drivers, mapped to the feature that needs them. * @@ -88,7 +113,12 @@ private function requiredValues(): array */ private function broadcastingValues(): array { - $connection = Configured::string('broadcasting.default', 'null'); + $connection = Configured::string('broadcasting.default'); + + if ($connection === null) { + return []; + } + $driver = Configured::string("broadcasting.connections.{$connection}.driver", $connection); $keys = match ($driver) { @@ -107,7 +137,12 @@ private function broadcastingValues(): array */ private function missingMercureValues(): array { - $connection = Configured::string('broadcasting.default', 'null'); + $connection = Configured::string('broadcasting.default'); + + if ($connection === null) { + return []; + } + $prefix = "broadcasting.connections.{$connection}"; if (Configured::string("{$prefix}.driver", $connection) !== 'mercure') { @@ -140,9 +175,9 @@ private function missingMercureValues(): array */ private function filesystemValues(): array { - $disk = Configured::string('filesystems.default', 'local'); + $disk = Configured::string('filesystems.default'); - if (Configured::string("filesystems.disks.{$disk}.driver", $disk) !== 's3') { + if ($disk === null || Configured::string("filesystems.disks.{$disk}.driver", $disk) !== 's3') { return []; } @@ -156,9 +191,16 @@ private function filesystemValues(): array */ private function loggingValues(): array { + $default = Configured::string('logging.default'); + + // The default log channel is optional; without one, LogManager writes to its emergency logger. + if ($default === null) { + return []; + } + $values = []; - foreach (ActiveDrivers::logChannels(Configured::string('logging.default', 'stack')) as $channel) { + foreach (ActiveDrivers::logChannels($default) as $channel) { $values = [...$values, ...$this->logChannelValues($channel)]; } @@ -204,9 +246,15 @@ private function logChannelValues(string $channel): array */ private function missingMailValues(): array { + $default = Configured::string('mail.default'); + + if ($default === null) { + return []; + } + $values = []; - foreach (ActiveDrivers::mailers(Configured::string('mail.default', 'log')) as $mailer) { + foreach (ActiveDrivers::mailers($default) as $mailer) { $values = [...$values, ...$this->missingMailerValues($mailer)]; } @@ -263,7 +311,12 @@ private function missingMailerValues(string $mailer): array */ private function queueValues(): array { - $connection = Configured::string('queue.default', 'database'); + $connection = Configured::string('queue.default'); + + if ($connection === null) { + return []; + } + $values = []; foreach (ActiveDrivers::queueConnections($connection) as $name) { diff --git a/tests/Doctor/Feature/Diagnostics/RequiredConfigurationValuesAreSetTest.php b/tests/Doctor/Feature/Diagnostics/RequiredConfigurationValuesAreSetTest.php index d0a616af9..17de03bac 100644 --- a/tests/Doctor/Feature/Diagnostics/RequiredConfigurationValuesAreSetTest.php +++ b/tests/Doctor/Feature/Diagnostics/RequiredConfigurationValuesAreSetTest.php @@ -19,6 +19,41 @@ public function testPassesWithTheDefaultConfiguration(): void $this->assertSame('pass', $result->status->value); } + #[DataProvider('requiredDefaults')] + public function testReportsBlankRequiredDefaultsInsteadOfAssumingADriver(string $key, string $feature): void + { + config([$key => '']); + + $result = (new RequiredConfigurationValuesAreSet)->check(); + + $this->assertSame('fail', $result->status->value); + $this->assertSame("- {$key} ({$feature})", $result->details); + } + + /** + * Provide required default keys and the features that need them. + */ + public static function requiredDefaults(): array + { + return [ + ['broadcasting.default', 'default broadcaster'], + ['filesystems.default', 'default filesystem disk'], + ['mail.default', 'default mailer'], + ['queue.default', 'default queue connection'], + ]; + } + + public function testDoesNotAssumeAStackChannelWithoutADefaultLogChannel(): void + { + config([ + 'logging.default' => '', + 'logging.channels.stack' => ['driver' => 'stack', 'channels' => ['slack']], + 'logging.channels.slack' => ['driver' => 'slack', 'url' => null], + ]); + + $this->assertSame('pass', (new RequiredConfigurationValuesAreSet)->check()->status->value); + } + public function testReportsMissingCredentialsForTheActivePusherBroadcastConnection(): void { config([ From 5156e475b0619a35276763ab4d30a1620821b1c3 Mon Sep 17 00:00:00 2001 From: Albert Chen Date: Mon, 2 Nov 2026 11:47:40 +0800 Subject: [PATCH 4/4] fix: explain Composer autoload failures without output When Composer exited unsuccessfully without writing to stdout or stderr, for example after being killed, the autoload check and fix reported an empty details string. Both paths now use Details::processOutput() and fall back to a message that includes Composer's exit code. --- .../Diagnostics/ComposerAutoloadIsValid.php | 18 ++++++++++++++++-- .../ComposerAutoloadIsValidTest.php | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/doctor/src/Diagnostics/ComposerAutoloadIsValid.php b/src/doctor/src/Diagnostics/ComposerAutoloadIsValid.php index 8bd116d2a..0d60c34f6 100644 --- a/src/doctor/src/Diagnostics/ComposerAutoloadIsValid.php +++ b/src/doctor/src/Diagnostics/ComposerAutoloadIsValid.php @@ -4,12 +4,14 @@ namespace Hypervel\Doctor\Diagnostics; +use Hypervel\Contracts\Process\ProcessResult; use Hypervel\Doctor\Contracts\Fixable; use Hypervel\Doctor\Diagnostic; use Hypervel\Doctor\Results\DiagnosticResult; use Hypervel\Doctor\Results\FixResult; use Hypervel\Doctor\Results\Message; use Hypervel\Doctor\Support\Composer; +use Hypervel\Doctor\Support\Details; use Hypervel\Foundation\PackageManifest; use Hypervel\Support\Facades\Process; @@ -70,7 +72,7 @@ public function check(): DiagnosticResult return $this->fail('invalid') ->fixable() - ->withDetails(trim($process->errorOutput() !== '' ? $process->errorOutput() : $process->output())); + ->withDetails($this->failureDetails($process)); } /** @@ -89,9 +91,21 @@ public function fix(DiagnosticResult $result, ?string $option = null): FixResult if (! $process->successful()) { return $this->fixFailed('regeneration-failed') - ->withDetails(trim($process->errorOutput() !== '' ? $process->errorOutput() : $process->output())); + ->withDetails($this->failureDetails($process)); } return $this->fixed('regenerated'); } + + /** + * Get the failure details for a Composer process. + */ + private function failureDetails(ProcessResult $process): string + { + return Details::processOutput( + $process->output(), + $process->errorOutput(), + "Composer exited with code {$process->exitCode()} without output.", + ); + } } diff --git a/tests/Doctor/Feature/Diagnostics/ComposerAutoloadIsValidTest.php b/tests/Doctor/Feature/Diagnostics/ComposerAutoloadIsValidTest.php index 3d1767e88..5fe77d19f 100644 --- a/tests/Doctor/Feature/Diagnostics/ComposerAutoloadIsValidTest.php +++ b/tests/Doctor/Feature/Diagnostics/ComposerAutoloadIsValidTest.php @@ -108,4 +108,23 @@ public function testRegeneratesComposerAutoloadFilesWhenFixed(): void $this->assertSame('pass', $fix->status->value); } + + public function testExplainsComposerFailuresWithoutOutput(): void + { + $basePath = $this->basePath; + mkdir($basePath . '/vendor'); + file_put_contents($basePath . '/composer.json', '{}'); + file_put_contents($basePath . '/vendor/autoload.php', ' Process::result(exitCode: 137), + ]); + + $check = (new ComposerAutoloadIsValid)->check(); + $fix = (new ComposerAutoloadIsValid)->fix($check); + + $this->assertSame('fail', $check->status->value); + $this->assertSame('Composer exited with code 137 without output.', $check->details); + $this->assertSame('Composer exited with code 137 without output.', $fix->details); + } }