fix: log a Proxmox error once, with one stack trace

CHANGELOG.md

@@ -5,6 +5,14 @@ This file is a running track of new features and fixes to each version of the pa
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project
 follows [Semantic Versioning](https://semver.org) guidelines.
 
+## Unreleased
+
+### Fixed
+
+- Fixed every error from Proxmox filling the log with three copies of the same stack trace -- about 256 lines per error,
+  every few seconds while a page polled a server whose VM was gone. An error is now logged once, with one trace, and its
+  message is Proxmox's own ("Proxmox returned 500: Configuration file ... does not exist").
+
 ## v4.7.0-rc.2
 
 ### Fixed

app/Exceptions/Repository/Proxmox/ProxmoxConnectionException.php

@@ -8,8 +8,22 @@
 
 class ProxmoxConnectionException extends RepositoryException
 {
-   public function __construct(Response $response, RequestException $exception)
-   {
-       parent::__construct($response->reason() . PHP_EOL . $exception->getMessage() . PHP_EOL . $exception->getTraceAsString(), $exception->getCode(), $exception);
-   }
+    /**
+     * The message is the HTTP status and Proxmox's own error, e.g. "Proxmox
+     * returned 500: Configuration file '…/109.conf' does not exist".
+     *
+     * It used to be the RequestException's message plus its full stack trace,
+     * with the RequestException chained as the previous exception -- so a logged
+     * error printed the trace three times: in the message, for this exception,
+     * and for the previous one, both thrown from the same line. The response is
+     * kept on the exception instead, for callers that need Proxmox's answer.
+     */
+    public function __construct(public readonly Response $response, RequestException $exception)
+    {
+        $error = trim((string) $response->json('message'))
+            ?: trim($response->reason())
+            ?: $exception->getMessage();
+
+        parent::__construct("Proxmox returned {$response->status()}: {$error}", $exception->getCode());
+    }
 }

tests/Unit/Exceptions/ProxmoxConnectionExceptionTest.php

@@ -0,0 +1,39 @@
+<?php
+
+use Convoy\Exceptions\Repository\Proxmox\ProxmoxConnectionException;
+use Convoy\Repositories\Proxmox\Server\ProxmoxConfigRepository;
+use Illuminate\Support\Facades\Http;
+
+function proxmoxErrorFor(array|string $body, int $status = 500): ProxmoxConnectionException
+{
+    Http::fake(['*' => Http::response($body, $status)]);
+    [$_, $_, $_, $server] = createServerModel();
+
+    try {
+        app(ProxmoxConfigRepository::class)->setServer($server)->getConfig();
+    } catch (ProxmoxConnectionException $e) {
+        return $e;
+    }
+
+    throw new RuntimeException('Expected a ProxmoxConnectionException');
+}
+
+// A logged Proxmox error used to print its stack trace three times: inside the
+// message, for this exception, and for the RequestException chained under it.
+it('logs one trace: no trace in the message, nothing chained', function () {
+    $e = proxmoxErrorFor(['data' => null, 'message' => "Configuration file 'nodes/pve/qemu-server/109.conf' does not exist\n"]);
+
+    expect($e->getMessage())
+        ->toBe("Proxmox returned 500: Configuration file 'nodes/pve/qemu-server/109.conf' does not exist")
+        ->not->toContain('#0 ')
+        ->and($e->getPrevious())->toBeNull()
+        ->and($e->response->status())->toBe(500);
+});
+
+it("falls back to the HTTP client's message when Proxmox sends no JSON", function () {
+    $e = proxmoxErrorFor("can't connect to 10.0.0.1:443 (connection refused)", 595);
+
+    expect($e->getMessage())
+        ->toContain("can't connect to 10.0.0.1:443")
+        ->not->toContain('#0 ');
+});