diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 2340ef67bd9d..465894aac8d4 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -91,15 +91,18 @@ class ErrorBuilder private: EvalState & state; ErrorInfo info; + hintformat message; - ErrorBuilder(EvalState & s, ErrorInfo && i): state(s), info(i) { } + ErrorBuilder(EvalState & s, hintformat && message) + : state(s), message(message) + { } public: template [[nodiscard, gnu::noinline]] static ErrorBuilder * create(EvalState & s, const Args & ... args) { - return new ErrorBuilder(s, ErrorInfo { .msg = hintfmt(args...) }); + return new ErrorBuilder(s, hintfmt(args...)); } [[nodiscard, gnu::noinline]] @@ -659,7 +662,7 @@ template void ErrorBuilder::debugThrow() { // NOTE: We always use the -LastTrace version as we push the new trace in withFrame() - state.debugThrowLastTrace(ErrorType(info)); + state.debugThrowLastTrace(ErrorType { info, message }); } } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index dec5818fcfc7..306ca9639983 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -74,19 +74,23 @@ namespace nix { static void dupAttr(const EvalState & state, const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos) { - throw ParseError({ - .msg = hintfmt("attribute '%1%' already defined at %2%", - showAttrPath(state.symbols, attrPath), state.positions[prevPos]), - .errPos = state.positions[pos] - }); + throw ParseError( + { + .errPos = state.positions[pos] + }, + hintfmt("attribute '%1%' already defined at %2%", + showAttrPath(state.symbols, attrPath), state.positions[prevPos]), + ); } static void dupAttr(const EvalState & state, Symbol attr, const PosIdx pos, const PosIdx prevPos) { - throw ParseError({ - .msg = hintfmt("attribute '%1%' already defined at %2%", state.symbols[attr], state.positions[prevPos]), - .errPos = state.positions[pos] - }); + throw ParseError( + { + .errPos = state.positions[pos] + }, + hintfmt("attribute '%1%' already defined at %2%", state.symbols[attr], state.positions[prevPos]), + ); } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 2e924c302956..fd72b3882cc9 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -144,10 +144,12 @@ static void fetchTree( if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) - state.debugThrowLastTrace(EvalError({ - .msg = hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), - .errPos = state.positions[pos] - })); + state.debugThrowLastTrace(EvalError( + { + .errPos = state.positions[pos] + }, + hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), + )); input = fetchers::Input::fromAttrs(std::move(attrs)); } else { @@ -463,10 +465,10 @@ static RegisterPrimOp primop_fetchGit({ builtins.fetchGit ./work-dir ``` - If the URL points to a local directory, and no `ref` or `rev` is - given, `fetchGit` will use the current content of the checked-out - files, even if they are not committed or added to Git's index. It will - only consider files added to the Git repository, as listed by `git ls-files`. + If the URL points to a local directory, and no `ref` or `rev` is + given, `fetchGit` will use the current content of the checked-out + files, even if they are not committed or added to Git's index. It will + only consider files added to the Git repository, as listed by `git ls-files`. )", .fun = prim_fetchGit, }); diff --git a/src/libexpr/value/context.hh b/src/libexpr/value/context.hh index 721563cbaef6..53095132a38e 100644 --- a/src/libexpr/value/context.hh +++ b/src/libexpr/value/context.hh @@ -21,7 +21,7 @@ public: { raw = raw_; auto hf = hintfmt(args...); - err.msg = hintfmt("Bad String Context element: %1%: %2%", normaltxt(hf.str()), raw); + message = hintfmt("Bad String Context element: %1%: %2%", normaltxt(hf.str()), raw); } }; diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 871f2f3be834..39b19a241719 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -12,7 +12,7 @@ SQLiteError::SQLiteError(const char *path, const char *errMsg, int errNo, int ex : Error(""), path(path), errMsg(errMsg), errNo(errNo), extendedErrNo(extendedErrNo), offset(offset) { auto offsetStr = (offset == -1) ? "" : "at offset " + std::to_string(offset) + ": "; - err.msg = hintfmt("%s: %s%s, %s (in '%s')", + message = hintfmt("%s: %s%s, %s (in '%s')", normaltxt(hf.str()), offsetStr, sqlite3_errstr(extendedErrNo), @@ -31,7 +31,7 @@ SQLiteError::SQLiteError(const char *path, const char *errMsg, int errNo, int ex if (err == SQLITE_BUSY || err == SQLITE_PROTOCOL) { auto exp = SQLiteBusy(path, errMsg, err, exterr, offset, std::move(hf)); - exp.err.msg = hintfmt( + exp.message = hintfmt( err == SQLITE_PROTOCOL ? "SQLite database '%s' is busy (SQLITE_PROTOCOL)" : "SQLite database '%s' is busy", @@ -247,9 +247,7 @@ void handleSQLiteBusy(const SQLiteBusy & e) if (now > lastWarned + 10) { lastWarned = now; - logWarning({ - .msg = hintfmt(e.what()) - }); + logWarning(ErrorInfo {}, hintfmt(e.what())); } /* Sleep for a while since retrying the transaction right away diff --git a/src/libutil/error.cc b/src/libutil/error.cc index e4f0d46778d2..0bbeb137f59f 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -21,13 +21,18 @@ const std::string & BaseError::calcWhat() const if (what_.has_value()) return *what_; else { - std::ostringstream oss; - showErrorInfo(oss, err, loggerSettings.showTrace); - what_ = oss.str(); + what_ = calcWhatUncached(); return *what_; } } +std::string Error::calcWhatUncached() const +{ + std::ostringstream oss; + showErrorInfo(oss, err, message, loggerSettings.showTrace); + return oss.str(); +} + std::optional ErrorInfo::programName = std::nullopt; std::ostream & operator <<(std::ostream & os, const hintformat & hf) @@ -150,7 +155,11 @@ static std::string indent(std::string_view indentFirst, std::string_view indentR return res; } -std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool showTrace) +std::ostream & showErrorInfo( + std::ostream & out, + const ErrorInfo & einfo, + std::function msg, + bool showTrace) { std::string prefix; switch (einfo.level) { @@ -331,7 +340,9 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s oss << "\n" << prefix; } - oss << einfo.msg << "\n"; + msg(oss); + + oss << "\n"; if (einfo.errPos) { oss << "\n" << ANSI_BLUE << "at " ANSI_WARNING << *einfo.errPos << ANSI_NORMAL << ":"; @@ -355,4 +366,19 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s return out; } + +std::ostream & showErrorInfo( + std::ostream & out, + const ErrorInfo & einfo, + hintformat msg, + bool showTrace) +{ + return showErrorInfo( + oss, + err, + [this](std::ostringstream & oss) { + oss << msg; + }, + showTrace); +} } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 0ebeaba61398..bd1dc802acd3 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -93,7 +93,6 @@ struct Trace { struct ErrorInfo { Verbosity level; - hintformat msg; std::shared_ptr errPos; std::list traces; @@ -102,7 +101,17 @@ struct ErrorInfo { static std::optional programName; }; -std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool showTrace); +std::ostream & showErrorInfo( + std::ostream & out, + const ErrorInfo & einfo, + std::function msg, + bool showTrace); +/* Convenience for the common case. */ +std::ostream & showErrorInfo( + std::ostream & out, + const ErrorInfo & einfo, + hintformat msg, + bool showTrace); /* BaseError should generally not be caught, as it has Interrupted as a subclass. Catch Error instead. */ @@ -113,32 +122,15 @@ protected: mutable std::optional what_; const std::string & calcWhat() const; - -public: - unsigned int status = 1; // exit status + virtual std::string calcWhatUncached() const = 0; BaseError(const BaseError &) = default; - template - BaseError(unsigned int status, const Args & ... args) - : err { .level = lvlError, .msg = hintfmt(args...) } + BaseError(unsigned int status, ErrorInfo && e) + : err(std::move(e)) , status(status) { } - template - explicit BaseError(const std::string & fs, const Args & ... args) - : err { .level = lvlError, .msg = hintfmt(fs, args...) } - { } - - template - BaseError(const Suggestions & sug, const Args & ... args) - : err { .level = lvlError, .msg = hintfmt(args...), .suggestions = sug } - { } - - BaseError(hintformat hint) - : err { .level = lvlError, .msg = hint } - { } - BaseError(ErrorInfo && e) : err(std::move(e)) { } @@ -147,6 +139,9 @@ public: : err(e) { } +public: + unsigned int status = 1; // exit status + #ifdef EXCEPTION_NEEDS_THROW_SPEC ~BaseError() throw () { }; const char * what() const throw () { return calcWhat().c_str(); } @@ -175,6 +170,44 @@ public: const ErrorInfo & info() { return err; }; }; +struct Error : BaseError +{ + hintformat message; + + Error(ErrorInfo && e, hintformat && message) + : BaseError(e), message(message) + { } + + Error(const ErrorInfo & e, const hintformat & message) + : BaseError(e), message(message) + { } + + template + Error(unsigned int status, const Args & ... args) + : BaseError(status, ErrorInfo { .level = lvlError }) + , message(hintfmt(args...)) + { } + + template + explicit Error(const std::string & fs, const Args & ... args) + : BaseError(ErrorInfo { .level = lvlError }) + , message(hintfmt(fs, args...)) + { } + + template + Error(const Suggestions & sug, const Args & ... args) + : BaseError(ErrorInfo { .level = lvlError, .suggestions = sug }) + , message(hintfmt(args...)) + { } + + Error(hintformat hint) + : BaseError(ErrorInfo { .level = lvlError }) + , message(hint) + { } + + std::string calcWhatUncached() const override; +}; + #define MakeError(newClass, superClass) \ class newClass : public superClass \ { \ @@ -182,7 +215,6 @@ public: using superClass::superClass; \ } -MakeError(Error, BaseError); MakeError(UsageError, Error); MakeError(UnimplementedError, Error); @@ -197,7 +229,7 @@ public: { errNo = errNo_; auto hf = hintfmt(args...); - err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo)); + message = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo)); } template diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 904ba6ebee48..92f03f70bf88 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -74,10 +74,10 @@ class SimpleLogger : public Logger writeToStderr(prefix + filterANSIEscapes(fs.s, !tty) + "\n"); } - void logEI(const ErrorInfo & ei) override + void logEI(const ErrorInfo & ei, hintformat msg) override { std::stringstream oss; - showErrorInfo(oss, ei, loggerSettings.showTrace.get()); + showErrorInfo(oss, ei, msg, loggerSettings.showTrace.get()); log(ei.level, oss.str()); } @@ -182,16 +182,16 @@ struct JSONLogger : Logger { write(json); } - void logEI(const ErrorInfo & ei) override + void logEI(const ErrorInfo & ei, hintformat message) override { std::ostringstream oss; - showErrorInfo(oss, ei, loggerSettings.showTrace.get()); + showErrorInfo(oss, ei, message, loggerSettings.showTrace.get()); nlohmann::json json; json["action"] = "msg"; json["level"] = ei.level; json["msg"] = oss.str(); - json["raw_msg"] = ei.msg.str(); + json["raw_msg"] = message.str(); to_json(json, ei.errPos); if (loggerSettings.showTrace.get() && !ei.traces.empty()) { diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 4642c49f7ed0..656fd055f97e 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -82,12 +82,12 @@ public: log(lvlInfo, fs); } - virtual void logEI(const ErrorInfo & ei) = 0; + virtual void logEI(const ErrorInfo & ei, hintformat msg) = 0; - void logEI(Verbosity lvl, ErrorInfo ei) + void logEI(Verbosity lvl, ErrorInfo ei, hintformat msg) { ei.level = lvl; - logEI(ei); + logEI(ei, msg); } virtual void warn(const std::string & msg); diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index c653db9d05e7..d28fadbe74f7 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -343,7 +343,7 @@ Sink & operator << (Sink & sink, const Error & ex) << "Error" << info.level << "Error" // removed - << info.msg.str() + << ex.message.str() << 0 // FIXME: info.errPos << info.traces.size(); for (auto & trace : info.traces) { @@ -412,10 +412,10 @@ Error readError(Source & source) assert(type == "Error"); auto level = (Verbosity) readInt(source); auto name = readString(source); // removed - auto msg = readString(source); + auto msg0 = readString(source); + auto msg = hintformat(std::move(format("%s") % msg0)); ErrorInfo info { .level = level, - .msg = hintformat(std::move(format("%s") % msg)), }; auto havePos = readNum(source); assert(havePos == 0); @@ -427,7 +427,7 @@ Error readError(Source & source) .hint = hintformat(std::move(format("%s") % readString(source))) }); } - return Error(std::move(info)); + return Error(std::move(info), msg); } diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index a22bccba113d..2ea9de6786b4 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -288,8 +288,8 @@ static void daemonLoop() } catch (Error & error) { auto ei = error.info(); // FIXME: add to trace? - ei.msg = hintfmt("error processing connection: %1%", ei.msg.str()); - logError(ei); + auto msg = hintfmt("error processing connection: %1%", error.message.str()); + logError(ei, msg); } } }