diff --git a/CHANGELOG.md b/CHANGELOG.md index d653acfd..2cffd275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **SABnzbd actions could report a false success.** `action()` (pause/resume/delete/speed-limit/add) defaulted an absent `status` key to `true`, so any unrecognised HTTP 200 — from a reverse proxy or a different SABnzbd version — was reported to the user as a successful action even though nothing happened. Pure SABnzbd actions always carry a `status` flag; its absence is now treated as failure and logged, matching `NzbgetClient`'s existing strictness. - **Whole-server lockup on Unraid when the data volume sits on a FUSE share (`/mnt/user`)**. PHP's native file session handler holds an exclusive `flock` on the session file for the entire request, and the dashboard fires ~6 widget fragments in parallel, so they all serialised on that one lock while their slow Radarr/Sonarr calls ran. On Unraid the session file lives on the shfs/FUSE share, where `flock` contention is expensive enough to peg every core and freeze the whole machine (mapping the volume to `/mnt/cache` "fixed" it only by bypassing FUSE). A new `SessionLockReleaseSubscriber` now closes the session right after authentication on read-only GET requests, releasing the lock immediately so the parallel fragments stop fighting over it. POSTs, the setup wizard and internal routes keep the session open and write normally. Unraid users should still map the data volume to `/mnt/cache/...` rather than `/mnt/user/...`. - **Gluetun integration with API key set, and incorrect endpoints.** The Gluetun client authenticated using `Authorization: Bearer `, but Gluetun expects it as `X-API-Key`, so it would return a 401 error when an API key is required. Additionally, referring to the older [Control Server Docs](https://github.com/qdm12/gluetun-wiki/blob/7025b1c0e4427d4477e47d4bbd2ef3f1b5c4da71/setup/advanced/control-server.md#openvpn-and-wireguard), WireGuard doesn't get its own endpoint, so the `/v1/wireguard/status` and `/v1/wireguard/portforwarded` calls were incorrect. The client now sends `X-API-Key` and uses the unified `/v1/vpn/status` and `/v1/portforward` endpoints, with the legacy `/v1/openvpn/` paths as a fallback. With that, the protocol selector in the settings becomes redundant and was removed. - **Radarr/Sonarr sidebar entry unusable with 4+ instances** ([#44](https://github.com/Shoshuo/Prismarr/issues/44)). The dropdown toggle shown for 4 or more instances of the same service used the native Bootstrap `data-bs-toggle`, but nothing in the app initializes Bootstrap's dropdown JS anymore — every other dropdown had already moved to a shared click-delegate. The triangle rendered but never opened, making every instance beyond the first three unreachable. The toggle now uses the same delegate as the rest of the app, and its label shows the active instance's name instead of staying stuck on the generic service name. diff --git a/symfony/src/Service/Media/Usenet/SabnzbdClient.php b/symfony/src/Service/Media/Usenet/SabnzbdClient.php index 933bedec..3a3dc819 100644 --- a/symfony/src/Service/Media/Usenet/SabnzbdClient.php +++ b/symfony/src/Service/Media/Usenet/SabnzbdClient.php @@ -320,7 +320,7 @@ private static function isOkStatus(int $code): bool * @param array $params * @return array */ - private function call(array $params): array + protected function call(array $params): array { if ($this->isBrokenOrDown()) return []; $this->lastError = null; @@ -381,8 +381,20 @@ private function action(array $params): bool if ($data === []) { return false; } - // version/queue/history don't carry a status flag; pure actions do. - return ($data['status'] ?? true) !== false; + + // Les actions pures renvoient toujours `status`. Son absence signale + // une réponse inattendue (proxy, version différente) : on échoue + // FERMÉ plutôt que d'annoncer un succès non vérifié à l'utilisateur. + if (!array_key_exists('status', $data)) { + $this->logger->warning('SABnzbd action response carried no status flag', [ + 'mode' => $params['mode'] ?? '?', + 'keys' => array_keys($data), + ]); + + return false; + } + + return $data['status'] !== false; } private function uploadNzb(string $content, string $name, ?string $category): bool diff --git a/symfony/tests/Service/SabnzbdActionResultTest.php b/symfony/tests/Service/SabnzbdActionResultTest.php new file mode 100644 index 00000000..4c0b4dc6 --- /dev/null +++ b/symfony/tests/Service/SabnzbdActionResultTest.php @@ -0,0 +1,67 @@ +clientReturning(['queue' => ['slots' => []]])->pauseAll(), + 'une réponse sans `status` ne doit pas valoir succès', + ); + } + + public function testActionWithExplicitTrueSucceeds(): void + { + self::assertTrue($this->clientReturning(['status' => true])->pauseAll()); + } + + public function testActionWithExplicitFalseFails(): void + { + self::assertFalse($this->clientReturning(['status' => false])->pauseAll()); + } + + private function clientReturning(array $payload): SabnzbdClient + { + // La sous-classe anonyme n'appelle pas le constructeur parent par + // défaut ; comme `action()` (non surchargé) touche désormais + // `$this->logger` quand `status` est absent, on le construit avec de + // vraies dépendances factices plutôt que de laisser les propriétés + // typed/readonly non initialisées. + return new class( + $this->createMock(ConfigService::class), + new NullLogger(), + $this->createMock(ServiceHealthCache::class), + $payload, + ) extends SabnzbdClient { + public function __construct( + ConfigService $config, + LoggerInterface $logger, + ServiceHealthCache $health, + private array $payload, + ) { + parent::__construct($config, $logger, $health); + } + + protected function call(array $params): array + { + return $this->payload; + } + }; + } +}