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/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/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/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/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/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); + } } 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 { 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/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([ 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']);