This adds a tinyformat::format_no_throw that (on error) returns the format string with the error message. format_no_throw is currently only used in logging, but can be used by other modules after this patch.
The fist commit reverts some style-changes that have been made to tinyformat, which is to be treated like a “subtree”, so style-fixes should go upstream and are not acceptable in Bitcoin Core.
tinyformat.h.
ACK on reverting the style changes.
1067- return oss.str();
1068+ return format(fmt.c_str(), args...);
1069+}
1070+
1071+template <typename... Args>
1072+std::string format_no_throw(const std::string& fmt, const Args&... args)
noexcept to make the “no throw” part of the contract and thus understandable also for the compiler :-)
std::out_of_range) are thrown?
There is really no shortcut here AFAIK: reading + reasoning would be required :-)
Do you see any places where e.g. std::out_of_range could be thrown that is reachable from format? Beyond the already handled format_error.
The contract of tinyformat is that format_error is the only exception thrown?
Stroustrup and Sutter:
E.12: Use noexcept when exiting a function because of a throw is impossible or unacceptable
format_no_throw seems like an ideal candidate for noexcept :-)
std::terminate. I think I will leave that for later, as I am not sure that immediate termination is better than offering the gui or daemon to catch the exception
I think it is unwise to use function naming (format_no_throw) which gives the caller the impression that the contract is that the function cannot throw if we’re not willing to back that with a noexcept guarantee.
I think we should either 1.) handle any non-fatal exception within format_no_throw and mark it noexcept, or 2.) remove the _no_throw suffix.
This function wraps format() to return error messages instead of throwing format_error. All other format() exceptions are thrown away.
139- } catch (tinyformat::format_error& fmterr) {
140- /* Original format string will have newline so don't add one here */
141- log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt;
142- }
143- LogInstance().LogPrintStr(log_msg);
144+ LogInstance().LogPrintStr(tfm::format_no_throw(fmt, args...));
I suggest making LogPrintf noexcept, but before doing so note that the following call chain is triggered here (also before this PR):
LogPrintStr → LogTimestampStr → FormatISO8601DateTime → strprintfGenerally strprintf may throw tinyformat::format_error.
In the case of the call FormatISO8601DateTime I don’t think that is possible since the format string is well formed. However, that is not understood by the compiler which currently must assume that FormatISO8601DateTime can throw.
I suggest handling the (non-reachable) tinyformat::format_error explicitly in FormatISO8601DateTime and then making that function noexcept too. (Should be done also for its companion FormatISO8601Date).
So in summary my suggestion is to:
tinyformat::format_error in FormatISO8601DateTime and FormatISO8601Datenoexcept to FormatISO8601DateTime, FormatISO8601Date and LogPrintf
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.