From 06792678383d54b0710cf6f0c5ead0417df36862 Mon Sep 17 00:00:00 2001
From: Eric Wang <37554696+ericwang401@users.noreply.github.com>
Date: Fri, 16 Oct 2026 00:49:35 +0000
Subject: [PATCH] fix: log a Proxmox error once, with one stack trace

ProxmoxConnectionException's message was the RequestException's message
plus its getTraceAsString(), and the RequestException was chained as the
previous exception. A logged Proxmox error therefore printed the trace
three times -- in the message, for the exception, and for the previous
one, thrown from the same line -- about 256 lines per error. A page
polling a server whose VM is gone wrote one of those every few seconds.

The message is now "Proxmox returned <status>: <Proxmox's error>", taken
from the response's JSON message, then its reason phrase, then the HTTP
client's message. Nothing is chained; the response is kept on the
exception as a public readonly property for callers that need it. A real
error from a node now logs as one 32-line entry with a single trace.

The ISO link check, which looks for Proxmox's "Can't connect to" in the
message, still finds it: that text is Proxmox's error, not the trace.
---
 CHANGELOG.md                                  |  8 ++++
 .../Proxmox/ProxmoxConnectionException.php    | 22 +++++++++--
 .../ProxmoxConnectionExceptionTest.php        | 39 +++++++++++++++++++
 3 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 tests/Unit/Exceptions/ProxmoxConnectionExceptionTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index deb96919c74..f2852cae1aa 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/app/Exceptions/Repository/Proxmox/ProxmoxConnectionException.php b/app/Exceptions/Repository/Proxmox/ProxmoxConnectionException.php
index 4a21a0e7580..31982ba8fab 100644
--- a/app/Exceptions/Repository/Proxmox/ProxmoxConnectionException.php
+++ b/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());
+    }
 }
diff --git a/tests/Unit/Exceptions/ProxmoxConnectionExceptionTest.php b/tests/Unit/Exceptions/ProxmoxConnectionExceptionTest.php
new file mode 100644
index 00000000000..e64e6d8cd21
--- /dev/null
+++ b/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 ');
+});