wroclaw-git-backups/nix
1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-28 22:01:15 +02:00
Code Activity

libexpr: Support structured error classes

Browse source 
While preparing PRs like #9753, I've had to change error messages in
dozens of code paths. It would be nice if instead of

    EvalError("expected 'boolean' but found '%1%'", showType(v))

we could write

    TypeError(v, "boolean")

or similar. Then, changing the error message could be a mechanical
refactor with the compiler pointing out places the constructor needs to
be changed, rather than the error-prone process of grepping through the
codebase. Structured errors would also help prevent the "same" error
from having multiple slightly different messages, and could be a first
step towards error codes / an error index.

This PR reworks the exception infrastructure in `libexpr` to
support exception types with different constructor signatures than
`BaseError`. Actually refactoring the exceptions to use structured data
will come in a future PR (this one is big enough already, as it has to
touch every exception in `libexpr`).

The core design is in `eval-error.hh`. Generally, errors like this:

    state.error("'%s' is not a string", getAttrPathStr())
      .debugThrow<TypeError>()

are transformed like this:

    state.error<TypeError>("'%s' is not a string", getAttrPathStr())
      .debugThrow()

The type annotation has moved from `ErrorBuilder::debugThrow` to
`EvalState::error`.
This commit is contained in:
Rebecca Turner 2024-01-22 17:08:29 -08:00
parent c62c21e29a
commit c6a89c1a16
No known key found for this signature in database
40 changed files with 653 additions and 545 deletions
Download patch file Download diff file Expand all files Collapse all files

50
src/libexpr/primops/context.cc
View file

@ -98,30 +98,30 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
auto contextSize = context.size();
if (contextSize != 1) {
throw EvalError({
.msg = hintfmt("context of string '%s' must have exactly one element, but has %d", *s, contextSize),
.errPos = state.positions[pos]
});
state.error<EvalError>(
"context of string '%s' must have exactly one element, but has %d",
*s,
contextSize
).atPos(pos).debugThrow();
}
NixStringContext context2 {
(NixStringContextElem { std::visit(overloaded {
[&](const NixStringContextElem::Opaque & c) -> NixStringContextElem::DrvDeep {
if (!c.path.isDerivation()) {
throw EvalError({
.msg = hintfmt("path '%s' is not a derivation",
state.store->printStorePath(c.path)),
.errPos = state.positions[pos],
});
state.error<EvalError>(
"path '%s' is not a derivation",
state.store->printStorePath(c.path)
).atPos(pos).debugThrow();
}
return NixStringContextElem::DrvDeep {
.drvPath = c.path,
};
},
[&](const NixStringContextElem::Built & c) -> NixStringContextElem::DrvDeep {
throw EvalError({
.msg = hintfmt("`addDrvOutputDependencies` can only act on derivations, not on a derivation output such as '%1%'", c.output),
.errPos = state.positions[pos],
});
state.error<EvalError>(
"`addDrvOutputDependencies` can only act on derivations, not on a derivation output such as '%1%'",
c.output
).atPos(pos).debugThrow();
},
[&](const NixStringContextElem::DrvDeep & c) -> NixStringContextElem::DrvDeep {
/* Reuse original item because we want this to be idempotent. */
@ -261,10 +261,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar
for (auto & i : *args[1]->attrs) {
const auto & name = state.symbols[i.name];
if (!state.store->isStorePath(name))
throw EvalError({
.msg = hintfmt("context key '%s' is not a store path", name),
.errPos = state.positions[i.pos]
});
state.error<EvalError>(
"context key '%s' is not a store path",
name
).atPos(i.pos).debugThrow();
auto namePath = state.store->parseStorePath(name);
if (!settings.readOnlyMode)
state.store->ensurePath(namePath);
@ -281,10 +281,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar
if (iter != i.value->attrs->end()) {
if (state.forceBool(*iter->value, iter->pos, "while evaluating the `allOutputs` attribute of a string context")) {
if (!isDerivation(name)) {
throw EvalError({
.msg = hintfmt("tried to add all-outputs context of %s, which is not a derivation, to a string", name),
.errPos = state.positions[i.pos]
});
state.error<EvalError>(
"tried to add all-outputs context of %s, which is not a derivation, to a string",
name
).atPos(i.pos).debugThrow();
}
context.emplace(NixStringContextElem::DrvDeep {
.drvPath = namePath,
@ -296,10 +296,10 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value * * ar
if (iter != i.value->attrs->end()) {
state.forceList(*iter->value, iter->pos, "while evaluating the `outputs` attribute of a string context");
if (iter->value->listSize() && !isDerivation(name)) {
throw EvalError({
.msg = hintfmt("tried to add derivation output context of %s, which is not a derivation, to a string", name),
.errPos = state.positions[i.pos]
});
state.error<EvalError>(
"tried to add derivation output context of %s, which is not a derivation, to a string",
name
).atPos(i.pos).debugThrow();
}
for (auto elem : iter->value->listItems()) {
auto outputName = state.forceStringNoCtx(*elem, iter->pos, "while evaluating an output name within a string context");

22
src/libexpr/primops/fetchClosure.cc
View file

@ -27,7 +27,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor
state.store->printStorePath(fromPath),
state.store->printStorePath(rewrittenPath),
state.store->printStorePath(*toPathMaybe)),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
if (!toPathMaybe)
throw Error({
@ -36,7 +36,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor
"Use this value for the 'toPath' attribute passed to 'fetchClosure'",
state.store->printStorePath(fromPath),
state.store->printStorePath(rewrittenPath)),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
@ -54,7 +54,7 @@ static void runFetchClosureWithRewrite(EvalState & state, const PosIdx pos, Stor
"The 'toPath' value '%s' is input-addressed, so it can't possibly be the result of rewriting to a content-addressed path.\n\n"
"Set 'toPath' to an empty string to make Nix report the correct content-addressed path.",
state.store->printStorePath(toPath)),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
@ -80,7 +80,7 @@ static void runFetchClosureWithContentAddressedPath(EvalState & state, const Pos
"to the 'fetchClosure' arguments.\n\n"
"Note that to ensure authenticity input-addressed store paths, users must configure a trusted binary cache public key on their systems. This is not needed for content-addressed paths.",
state.store->printStorePath(fromPath)),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
@ -103,7 +103,7 @@ static void runFetchClosureWithInputAddressedPath(EvalState & state, const PosId
"The store object referred to by 'fromPath' at '%s' is not input-addressed, but 'inputAddressed' is set to 'true'.\n\n"
"Remove the 'inputAddressed' attribute (it defaults to 'false') to expect 'fromPath' to be content-addressed",
state.store->printStorePath(fromPath)),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
@ -154,14 +154,14 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
else
throw Error({
.msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attrName),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
if (!fromPath)
throw Error({
.msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromPath"),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
bool inputAddressed = inputAddressedMaybe.value_or(false);
@ -172,14 +172,14 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
.msg = hintfmt("attribute '%s' is set to true, but '%s' is also set. Please remove one of them",
"inputAddressed",
"toPath"),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
}
if (!fromStoreUrl)
throw Error({
.msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
auto parsedURL = parseURL(*fromStoreUrl);
@ -189,13 +189,13 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg
!(getEnv("_NIX_IN_TEST").has_value() && parsedURL.scheme == "file"))
throw Error({
.msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
if (!parsedURL.query.empty())
throw Error({
.msg = hintfmt("'fetchClosure' does not support URL query parameters (in '%s')", *fromStoreUrl),
.errPos = state.positions[pos]
.pos = state.positions[pos]
});
auto fromStore = openStore(parsedURL.to_string());

10
src/libexpr/primops/fetchMercurial.cc
View file

@ -38,17 +38,11 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a
else if (n == "name")
name = state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `name` attribute passed to builtins.fetchMercurial");
else
throw EvalError({
.msg = hintfmt("unsupported argument '%s' to 'fetchMercurial'", state.symbols[attr.name]),
.errPos = state.positions[attr.pos]
});
state.error<EvalError>("unsupported argument '%s' to 'fetchMercurial'", state.symbols[attr.name]).atPos(attr.pos).debugThrow();
}
if (url.empty())
throw EvalError({
.msg = hintfmt("'url' argument required"),
.errPos = state.positions[pos]
});
state.error<EvalError>("'url' argument required").atPos(pos).debugThrow();
} else
url = state.coerceToString(pos, *args[0], context,

68
src/libexpr/primops/fetchTree.cc
View file

@ -100,16 +100,14 @@ static void fetchTree(
if (auto aType = args[0]->attrs->get(state.sType)) {
if (type)
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("unexpected attribute 'type'"),
.errPos = state.positions[pos]
}));
state.error<EvalError>(
"unexpected attribute 'type'"
).atPos(pos).debugThrow();
type = state.forceStringNoCtx(*aType->value, aType->pos, "while evaluating the `type` attribute passed to builtins.fetchTree");
} else if (!type)
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("attribute 'type' is missing in call to 'fetchTree'"),
.errPos = state.positions[pos]
}));
state.error<EvalError>(
"attribute 'type' is missing in call to 'fetchTree'"
).atPos(pos).debugThrow();
attrs.emplace("type", type.value());
@ -132,8 +130,8 @@ static void fetchTree(
attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, context).dump());
}
else
state.debugThrowLastTrace(TypeError("fetchTree argument '%s' is %s while a string, Boolean or integer is expected",
state.symbols[attr.name], showType(*attr.value)));
state.error<TypeError>("fetchTree argument '%s' is %s while a string, Boolean or integer is expected",
state.symbols[attr.name], showType(*attr.value)).debugThrow();
}
if (params.isFetchGit && !attrs.contains("exportIgnore") && (!attrs.contains("submodules") || !*fetchers::maybeGetBoolAttr(attrs, "submodules"))) {
@ -142,10 +140,9 @@ 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.error<EvalError>(
"attribute 'name' isn’t supported in call to 'fetchTree'"
).atPos(pos).debugThrow();
input = fetchers::Input::fromAttrs(std::move(attrs));
} else {
@ -163,10 +160,9 @@ static void fetchTree(
input = fetchers::Input::fromAttrs(std::move(attrs));
} else {
if (!experimentalFeatureSettings.isEnabled(Xp::Flakes))
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("passing a string argument to 'fetchTree' requires the 'flakes' experimental feature"),
.errPos = state.positions[pos]
}));
state.error<EvalError>(
"passing a string argument to 'fetchTree' requires the 'flakes' experimental feature"
).atPos(pos).debugThrow();
input = fetchers::Input::fromURL(url);
}
}
@ -175,10 +171,14 @@ static void fetchTree(
input = lookupInRegistries(state.store, input).first;
if (evalSettings.pureEval && !input.isLocked()) {
auto fetcher = "fetchTree";
if (params.isFetchGit)
state.debugThrowLastTrace(EvalError("in pure evaluation mode, 'fetchGit' requires a locked input, at %s", state.positions[pos]));
else
state.debugThrowLastTrace(EvalError("in pure evaluation mode, 'fetchTree' requires a locked input, at %s", state.positions[pos]));
fetcher = "fetchGit";
state.error<EvalError>(
"in pure evaluation mode, %s requires a locked input",
fetcher
).atPos(pos).debugThrow();
}
state.checkURI(input.toURLString());
@ -432,17 +432,13 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v
else if (n == "name")
name = state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the name of the content we should fetch");
else
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("unsupported argument '%s' to '%s'", n, who),
.errPos = state.positions[attr.pos]
}));
state.error<EvalError>("unsupported argument '%s' to '%s'", n, who)
.atPos(pos).debugThrow();
}
if (!url)
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("'url' argument required"),
.errPos = state.positions[pos]
}));
state.error<EvalError>(
"'url' argument required").atPos(pos).debugThrow();
} else
url = state.forceStringNoCtx(*args[0], pos, "while evaluating the url we should fetch");
@ -455,7 +451,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v
name = baseNameOf(*url);
if (evalSettings.pureEval && !expectedHash)
state.debugThrowLastTrace(EvalError("in pure evaluation mode, '%s' requires a 'sha256' argument", who));
state.error<EvalError>("in pure evaluation mode, '%s' requires a 'sha256' argument", who).atPos(pos).debugThrow();
// early exit if pinned and already in the store
if (expectedHash && expectedHash->algo == HashAlgorithm::SHA256) {
@ -484,9 +480,15 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v
auto hash = unpack
? state.store->queryPathInfo(storePath)->narHash
: hashFile(HashAlgorithm::SHA256, state.store->toRealPath(storePath));
if (hash != *expectedHash)
state.debugThrowLastTrace(EvalError((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n specified: %s\n got: %s",
*url, expectedHash->to_string(HashFormat::Nix32, true), hash.to_string(HashFormat::Nix32, true)));
if (hash != *expectedHash) {
state.error<EvalError>(
"hash mismatch in file downloaded from '%s':\n specified: %s\n got: %s",
*url,
expectedHash->to_string(HashFormat::Nix32, true),
hash.to_string(HashFormat::Nix32, true)
).withExitStatus(102)
.debugThrow();
}
}
state.allowAndSetStorePathString(storePath, v);

5
src/libexpr/primops/fromTOML.cc
View file

@ -83,10 +83,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value * * args, V
try {
visit(val, toml::parse(tomlStream, "fromTOML" /* the "filename" */));
} catch (std::exception & e) { // TODO: toml::syntax_error
throw EvalError({
.msg = hintfmt("while parsing a TOML string: %s", e.what()),
.errPos = state.positions[pos]
});
state.error<EvalError>("while parsing TOML: %s", e.what()).atPos(pos).debugThrow();
}
}