Skip to content

Commit

Permalink
Progress towards issue NixOS#7865
Browse files Browse the repository at this point in the history
  • Loading branch information
Ericson2314 committed May 10, 2023
1 parent f60b215 commit c758950
Show file tree
Hide file tree
Showing 32 changed files with 223 additions and 119 deletions.
2 changes: 1 addition & 1 deletion src/libcmd/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ void NixRepl::mainLoop()
try {
createDirs(dirOf(historyFile));
} catch (SysError & e) {
logWarning(e.info());
logExWarning(e);
}
#ifndef READLINE
el_hist_size = 1000;
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr &
.pos = error->info().errPos ? error->info().errPos : static_cast<std::shared_ptr<AbstractPos>>(positions[expr.getPos()]),
.expr = expr,
.env = env,
.hint = error->info().msg,
.hint = error->message,
.isError = true
})
: nullptr;
Expand Down
9 changes: 6 additions & 3 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,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<typename... Args>
[[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]]
Expand Down Expand Up @@ -756,7 +759,7 @@ template<class ErrorType>
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 });
}

}
Expand Down
18 changes: 9 additions & 9 deletions src/libexpr/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace nix {
Expr * result;
SourcePath basePath;
PosTable::Origin origin;
std::optional<ErrorInfo> error;
std::optional<ParseError> error;
};

struct ParserFormals {
Expand Down Expand Up @@ -292,10 +292,10 @@ static inline PosIdx makeCurPos(const YYLTYPE & loc, ParseData * data)

void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * error)
{
data->error = {
data->error = ParseError {{
.msg = hintfmt(error),
.errPos = data->state.positions[makeCurPos(*loc, data)]
};
.errPos = data->state.positions[makeCurPos(*loc, data)],
}};
}


Expand Down Expand Up @@ -667,7 +667,7 @@ Expr * EvalState::parse(
int res = yyparse(scanner, &data);
yylex_destroy(scanner);

if (res) throw ParseError(data.error.value());
if (res) throw data.error.value();

data.result->bindVars(*this, staticEnv);

Expand Down Expand Up @@ -796,9 +796,9 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
store, EvalSettings::resolvePseudoUrl(elem.second), "source", false).first.storePath;
res = { true, store->toRealPath(storePath) };
} catch (FileTransferError & e) {
logWarning({
logExWarning(Error{{
.msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second)
});
}});
res = { false, "" };
}
}
Expand All @@ -816,9 +816,9 @@ std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathEl
if (pathExists(path))
res = { true, path };
else {
logWarning({
logExWarning(Error{{
.msg = hintfmt("Nix search path entry '%1%' does not exist, ignoring", elem.second)
});
}});
res = { false, "" };
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ static RegisterPrimOp primop_break({
.fun = [](EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
if (state.debugRepl && !state.debugTraces.empty()) {
auto error = Error(ErrorInfo {
auto error = Error({
.level = lvlInfo,
.msg = hintfmt("breakpoint reached"),
.errPos = state.positions[pos],
Expand All @@ -755,7 +755,7 @@ static RegisterPrimOp primop_break({

if (state.debugQuit) {
// If the user elects to quit the repl, throw an exception.
throw Error(ErrorInfo{
throw Error({
.level = lvlInfo,
.msg = hintfmt("quit the debugger"),
.errPos = nullptr,
Expand Down
14 changes: 7 additions & 7 deletions src/libexpr/tests/error_traces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace nix {
throw;
}
} catch (BaseError & e) {
ASSERT_EQ(PrintToString(e.info().msg),
ASSERT_EQ(PrintToString(e.message),
PrintToString(hintfmt("Not much")));
auto trace = e.info().traces.rbegin();
ASSERT_EQ(e.info().traces.size(), 2);
Expand Down Expand Up @@ -61,16 +61,16 @@ namespace nix {
}
}

#define ASSERT_TRACE1(args, type, message) \
#define ASSERT_TRACE1(args, type, _message) \
ASSERT_THROW( \
std::string expr(args); \
std::string name = expr.substr(0, expr.find(" ")); \
try { \
Value v = eval("builtins." args); \
state.forceValueDeep(v); \
} catch (BaseError & e) { \
ASSERT_EQ(PrintToString(e.info().msg), \
PrintToString(message)); \
ASSERT_EQ(PrintToString(e.message), \
PrintToString(_message)); \
ASSERT_EQ(e.info().traces.size(), 1) << "while testing " args << std::endl << e.what(); \
auto trace = e.info().traces.rbegin(); \
ASSERT_EQ(PrintToString(trace->hint), \
Expand All @@ -80,16 +80,16 @@ namespace nix {
, type \
)

#define ASSERT_TRACE2(args, type, message, context) \
#define ASSERT_TRACE2(args, type, _message, context) \
ASSERT_THROW( \
std::string expr(args); \
std::string name = expr.substr(0, expr.find(" ")); \
try { \
Value v = eval("builtins." args); \
state.forceValueDeep(v); \
} catch (BaseError & e) { \
ASSERT_EQ(PrintToString(e.info().msg), \
PrintToString(message)); \
ASSERT_EQ(PrintToString(e.message), \
PrintToString(_message)); \
ASSERT_EQ(e.info().traces.size(), 2) << "while testing " args << std::endl << e.what(); \
auto trace = e.info().traces.rbegin(); \
ASSERT_EQ(PrintToString(trace->hint), \
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/tests/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ namespace nix {
oss << s << std::endl;
}

void logEI(const ErrorInfo & ei) override {
showErrorInfo(oss, ei, loggerSettings.showTrace.get());
void logEI(const ErrorInfo & ei, hintformat msg) override {
showErrorInfo(oss, ei, msg, loggerSettings.showTrace.get());
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/value/context.hh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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);
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/libmain/progress-bar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,12 @@ class ProgressBar : public Logger
log(*state, lvl, s);
}

void logEI(const ErrorInfo & ei) override
void logEI(const ErrorInfo & ei, hintformat msg) override
{
auto state(state_.lock());

std::stringstream oss;
showErrorInfo(oss, ei, loggerSettings.showTrace.get());
showErrorInfo(oss, ei, msg, loggerSettings.showTrace.get());

log(*state, ei.level, oss.str());
}
Expand Down
4 changes: 2 additions & 2 deletions src/libmain/shared.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ int handleExceptions(const std::string & programName, std::function<void()> fun)
} catch (Exit & e) {
return e.status;
} catch (UsageError & e) {
logError(e.info());
logExError(e);
printError("Try '%1% --help' for more information.", programName);
return 1;
} catch (BaseError & e) {
logError(e.info());
logExError(e);
return e.status;
} catch (std::bad_alloc & e) {
printError(error + "out of memory");
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink)
try {
getFile(info->url, *decompressor);
} catch (NoSuchBinaryCacheFile & e) {
throw SubstituteGone(std::move(e.info()));
throw SubstituteGone(std::move(e.info()), e.message);
}

decompressor->finish();
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ void DerivationGoal::done(
{
buildResult.status = status;
if (ex)
buildResult.errorMsg = fmt("%s", normaltxt(ex->info().msg));
buildResult.errorMsg = fmt("%s", normaltxt(ex->message));
if (buildResult.status == BuildResult::TimedOut)
worker.timedOut = true;
if (buildResult.status == BuildResult::PermanentFailure)
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/build/entry-points.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
for (auto & i : goals) {
if (i->ex) {
if (ex)
logError(i->ex->info());
logExError(*i->ex);
else
ex = std::move(i->ex);
}
Expand All @@ -34,7 +34,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
ex->status = worker.exitStatus();
throw std::move(*ex);
} else if (!failed.empty()) {
if (ex) logError(ex->info());
if (ex) logExError(*ex);
throw Error(worker.exitStatus(), "build of %s failed", showPaths(failed));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/build/goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void Goal::amDone(ExitCode result, std::optional<Error> ex)

if (ex) {
if (!waiters.empty())
logError(ex->info());
logExError(*ex);
else
this->ex = std::move(*ex);
}
Expand Down
6 changes: 3 additions & 3 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ void handleDiffHook(
if (diffRes.second != "")
printError(chomp(diffRes.second));
} catch (Error & error) {
ErrorInfo ei = error.info();
Error e = error;
// FIXME: wrap errors.
ei.msg = hintfmt("diff hook execution failed: %s", ei.msg.str());
logError(ei);
e.message = hintfmt("diff hook execution failed: %s", e.message.str());
logExError(e);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/build/substitution-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void PathSubstitutionGoal::tryNext()
throw;
} catch (Error & e) {
if (settings.tryFallback) {
logError(e.info());
logExError(e);
tryNext();
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/daemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ struct TunnelLogger : public Logger
enqueueMsg(buf.s);
}

void logEI(const ErrorInfo & ei) override
void logEI(const ErrorInfo & ei, hintformat msg) override
{
if (ei.level > verbosity) return;

std::stringstream oss;
showErrorInfo(oss, ei, false);
showErrorInfo(oss, ei, msg, false);

StringSink buf;
buf << STDERR_NEXT << oss.str();
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/filetransfer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -870,9 +870,9 @@ FileTransferError::FileTransferError(FileTransfer::Error error, std::optional<st
// to print different messages for different verbosity levels. For now
// we add some heuristics for detecting when we want to show the response.
if (response && (response->size() < 1024 || response->find("<html>") != std::string::npos))
err.msg = hintfmt("%1%\n\nresponse body:\n\n%2%", normaltxt(hf.str()), chomp(*response));
message = hintfmt("%1%\n\nresponse body:\n\n%2%", normaltxt(hf.str()), chomp(*response));
else
err.msg = hf;
message = hf;
}

}
4 changes: 2 additions & 2 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
/* It's possible that the path got GC'ed, so ignore
errors on invalid paths. */
if (isValidPath(i))
logError(e.info());
logExError(e);
else
warn(e.msg());
errors = true;
Expand Down Expand Up @@ -1629,7 +1629,7 @@ void LocalStore::verifyPath(const Path & pathS, const StringSet & store,
try {
repairPath(path);
} catch (Error & e) {
logWarning(e.info());
logExWarning(e);
errors = true;
}
else errors = true;
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/remote-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
} catch (Error & e) {
// Ugly backwards compatibility hack.
if (e.msg().find("is not valid") != std::string::npos)
throw InvalidPath(std::move(e.info()));
throw InvalidPath(std::move(e.info()), std::move(e.message));
throw;
}
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) {
Expand Down
8 changes: 3 additions & 5 deletions src/libstore/sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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",
Expand Down Expand Up @@ -244,9 +244,7 @@ void handleSQLiteBusy(const SQLiteBusy & e, time_t & nextWarning)
time_t now = time(0);
if (now > nextWarning) {
nextWarning = now + 10;
logWarning({
.msg = hintfmt(e.what())
});
logExWarning(e);
}

/* Sleep for a while since retrying the transaction right away
Expand Down
6 changes: 3 additions & 3 deletions src/libstore/store-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, Substituta
} catch (SubstituterDisabled &) {
} catch (Error & e) {
if (settings.tryFallback)
logError(e.info());
logExError(e);
else
throw;
}
Expand Down Expand Up @@ -794,7 +794,7 @@ void Store::substitutePaths(const StorePathSet & paths)
for (auto & p : willSubstitute) subs.push_back(DerivedPath::Opaque{p});
buildPaths(subs);
} catch (Error & e) {
logWarning(e.info());
logExWarning(e);
}
}

Expand Down Expand Up @@ -1504,7 +1504,7 @@ std::list<ref<Store>> getDefaultSubstituters()
try {
stores.push_back(openStore(uri));
} catch (Error & e) {
logWarning(e.info());
logExWarning(e);
}
};

Expand Down
Loading

0 comments on commit c758950

Please sign in to comment.