Skip to content

Commit

Permalink
lib/errorlogger.cpp: Improve conversion from InternalError to ErrorMe…
Browse files Browse the repository at this point in the history
…ssage

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.
  • Loading branch information
mvds00 committed Dec 23, 2023
1 parent 5a222b8 commit 5df3d7e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 8 additions & 1 deletion lib/errorlogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 5df3d7e

Please sign in to comment.