From 5df3d7e26ab3d3b314504ae461072a96ddf405ed Mon Sep 17 00:00:00 2001
From: Maarten van der Schrieck <maarten@thingsconnected.nl>
Date: Sat, 23 Dec 2023 15:44:51 +0100
Subject: [PATCH] lib/errorlogger.cpp: Improve conversion from InternalError to
 ErrorMessage

ErrorMessage::setmsg() assumes that the supplied message string is a
combination of a short message and a verbose message, separated by a newline.

However, InternalError does not use that format; when converting from
InternalError to ErrorMessage, the short message will receive the first line,
and the verbose message will receive the lines that follow.

This approach may lead to obscure messages during verbose logging since the
informative first line is lost.

This is solved by setting the verbose message to the complete original error
string.

A concrete example is the failure of an addon to produce output starting with
'{', which is reported as an InternalError. The clarity of this message is
also improved with this patch, as the "Failure to execute" criterion was not
clear from the message.
---
 lib/cppcheck.cpp    | 2 +-
 lib/errorlogger.cpp | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp
index 9f912697db8c..23d0cafea047 100644
--- a/lib/cppcheck.cpp
+++ b/lib/cppcheck.cpp
@@ -310,7 +310,7 @@ static std::vector<picojson::value> executeAddon(const AddonInfo &addonInfo,
             //std::cout << "addon '" << addonInfo.name <<  "' result is not a JSON" << std::endl;
 
             result.erase(result.find_last_not_of('\n') + 1, std::string::npos); // Remove trailing newlines
-            throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. " + result);
+            throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. Expected '{' but got '" + line[0] + "' as first character. Output was: '" + result + "'");
         }
 
         //std::cout << "addon '" << addonInfo.name <<  "' result is " << line << std::endl;
diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp
index fbb635c691df..112f61d5a975 100644
--- a/lib/errorlogger.cpp
+++ b/lib/errorlogger.cpp
@@ -252,12 +252,19 @@ ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError,
             locationList.push_back(std::move(loc));
         }
     }
+    std::string prefixedMessage = (msg.empty() ? "" : (msg + ": ")) + internalError.errorMessage;
     ErrorMessage errmsg(std::move(locationList),
                         tokenList ? tokenList->getSourceFilePath() : filename,
                         Severity::error,
-                        (msg.empty() ? "" : (msg + ": ")) + internalError.errorMessage,
+                        prefixedMessage,
                         internalError.id,
                         Certainty::normal);
+
+    // ErrorMessage assumes a message looking like "short message\nverbose message".
+    // The internal error may not reflect that format, leading to obscure messages if verbose
+    // logging is requested: the first (often most informative) line will simply vanish.
+    // Solve this by setting mVerboseMessage to the complete original message:
+    errmsg.mVerboseMessage = prefixedMessage;
     // TODO: find a better way
     if (!internalError.details.empty())
         errmsg.mVerboseMessage = errmsg.mVerboseMessage + ": " + internalError.details;