fix: three errors that ended with the wrong exit code
Found compiling the environment-variable table for AGENTS.md: - A REVIEW_GATES_MEM_RESERVE_MB that is no number threw std::invalid_argument, which nothing caught: the process aborted where it should end with a ValueError, exit 1. nan and inf were an unchecked cast. They raise ValueError / OverflowError now; a value past int64 saturates. - An env file that is not UTF-8: `run` ended with exit 1 where its preflight should stop it (exit 3), and `seats` parsed the raw bytes. Both read it as strict UTF-8 text now. - A broker address with a port past 65535 was cut to 16 bits; it is an OverflowError now, uncaught (exit 1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
b092d98697
commit
53b7d6de31
9 changed files with 58 additions and 14 deletions
|
|
@ -515,9 +515,9 @@ inventory unit, against a frozen commit range.)";
|
||||||
if (envFile.empty() && fs::exists(".env", error)) envFile = ".env";
|
if (envFile.empty() && fs::exists(".env", error)) envFile = ".env";
|
||||||
std::map<std::string, std::string> env = process;
|
std::map<std::string, std::string> env = process;
|
||||||
if (!envFile.empty()) {
|
if (!envFile.empty()) {
|
||||||
const std::optional<std::string> text = Platform::ReadFile(envFile);
|
// The env file, read as text: one that cannot be read, or is not
|
||||||
if (!text) throw Dyn::Error("FileNotFoundError", std::format("[Errno 2] No such file or directory: {}", Dyn::Repr(envFile)));
|
// UTF-8, ends the command.
|
||||||
for (const auto& [name, value] : Seats::SeatTokensFromEnvFile(*text)) env[name] = value;
|
for (const auto& [name, value] : Seats::SeatTokensFromEnvFile(Dyn::ReadText(envFile))) env[name] = value;
|
||||||
}
|
}
|
||||||
std::vector<SeatJob> jobs;
|
std::vector<SeatJob> jobs;
|
||||||
for (const auto& [seat, token] : Seats::SeatTokensFromEnv(env)) jobs.push_back({seat, token, false});
|
for (const auto& [seat, token] : Seats::SeatTokensFromEnv(env)) jobs.push_back({seat, token, false});
|
||||||
|
|
|
||||||
|
|
@ -20,8 +20,15 @@ namespace Crafter::Review::Memory {
|
||||||
|
|
||||||
std::int64_t Reserve(const std::optional<std::string>& configured, const fs::path& path) {
|
std::int64_t Reserve(const std::optional<std::string>& configured, const fs::path& path) {
|
||||||
if (configured && !configured->empty()) {
|
if (configured && !configured->empty()) {
|
||||||
|
// A number, truncated toward zero; ValueError or OverflowError for
|
||||||
|
// what is none. A reserve past int64 saturates: it holds every MB
|
||||||
|
// either way.
|
||||||
const std::optional<double> value = Dyn::ParseFloat(*configured);
|
const std::optional<double> value = Dyn::ParseFloat(*configured);
|
||||||
if (!value) throw std::invalid_argument(Dyn::FloatError(*configured));
|
if (!value) throw Dyn::Error("ValueError", Dyn::FloatError(*configured));
|
||||||
|
if (std::isnan(*value)) throw Dyn::Error("ValueError", "cannot convert float NaN to integer");
|
||||||
|
if (std::isinf(*value)) throw Dyn::Error("OverflowError", "cannot convert float infinity to integer");
|
||||||
|
if (*value >= 0x1p63) return std::numeric_limits<std::int64_t>::max();
|
||||||
|
if (*value < -0x1p63) return std::numeric_limits<std::int64_t>::min();
|
||||||
return static_cast<std::int64_t>(*value);
|
return static_cast<std::int64_t>(*value);
|
||||||
}
|
}
|
||||||
const std::map<std::string, std::int64_t> info = Platform::MemInfo(path);
|
const std::map<std::string, std::int64_t> info = Platform::MemInfo(path);
|
||||||
|
|
|
||||||
|
|
@ -1120,14 +1120,17 @@ namespace Crafter::Review::PgWire {
|
||||||
const std::size_t colon = where.rfind(':');
|
const std::size_t colon = where.rfind(':');
|
||||||
const std::string host(colon == std::string_view::npos ? std::string_view() : where.substr(0, colon));
|
const std::string host(colon == std::string_view::npos ? std::string_view() : where.substr(0, colon));
|
||||||
const std::string portText(colon == std::string_view::npos ? where : where.substr(colon + 1));
|
const std::string portText(colon == std::string_view::npos ? where : where.substr(colon + 1));
|
||||||
|
const std::optional<std::int64_t> port = Dyn::ParseInt(portText);
|
||||||
|
if (!port) {
|
||||||
|
out(std::format("{} - the driver's SQL broker does not answer ({})", NoDatabase, Dyn::IntError(portText)));
|
||||||
|
return ExitNoDb;
|
||||||
|
}
|
||||||
|
// socket.create_connection raised OverflowError for it, which
|
||||||
|
// call_broker did not catch: the helper ended in a traceback.
|
||||||
|
if (*port < 0 || *port > 65535) throw Dyn::Error("OverflowError", "getsockaddrarg: port must be 0-65535.");
|
||||||
std::unique_ptr<ClientTCP> socket;
|
std::unique_ptr<ClientTCP> socket;
|
||||||
try {
|
try {
|
||||||
const std::optional<std::int64_t> port = Dyn::ParseInt(portText);
|
|
||||||
if (!port) throw std::invalid_argument(Dyn::IntError(portText));
|
|
||||||
socket = std::make_unique<ClientTCP>(host, static_cast<std::uint16_t>(*port), std::chrono::seconds(5));
|
socket = std::make_unique<ClientTCP>(host, static_cast<std::uint16_t>(*port), std::chrono::seconds(5));
|
||||||
} catch (const std::invalid_argument& error) {
|
|
||||||
out(std::format("{} - the driver's SQL broker does not answer ({})", NoDatabase, error.what()));
|
|
||||||
return ExitNoDb;
|
|
||||||
} catch (const std::exception& error) {
|
} catch (const std::exception& error) {
|
||||||
out(std::format("{} - the driver's SQL broker does not answer ({})", NoDatabase, ConnectErrorText(error)));
|
out(std::format("{} - the driver's SQL broker does not answer ({})", NoDatabase, ConnectErrorText(error)));
|
||||||
return ExitNoDb;
|
return ExitNoDb;
|
||||||
|
|
|
||||||
|
|
@ -620,7 +620,10 @@ namespace Crafter::Review::Run {
|
||||||
} catch (const std::invalid_argument& e) {
|
} catch (const std::invalid_argument& e) {
|
||||||
Die(e.what());
|
Die(e.what());
|
||||||
} catch (const Dyn::Error& e) {
|
} catch (const Dyn::Error& e) {
|
||||||
if (e.Kind() != "FileNotFoundError" && e.Kind() != "PermissionError" && e.Kind() != "IsADirectoryError" && e.Kind() != "OSError") throw;
|
// The errors that mean the file could not be read: the OS errors, a
|
||||||
|
// file that is not UTF-8 (UnicodeDecodeError), and ValueError.
|
||||||
|
constexpr std::array<std::string_view, 6> Caught = {"FileNotFoundError", "PermissionError", "IsADirectoryError", "OSError", "UnicodeDecodeError", "ValueError"};
|
||||||
|
if (!std::ranges::contains(Caught, e.Kind())) throw;
|
||||||
Die(e.what());
|
Die(e.what());
|
||||||
}
|
}
|
||||||
// The board (2026-10-31): the other drivers on this machine, what they
|
// The board (2026-10-31): the other drivers on this machine, what they
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,8 @@ export namespace Crafter::Review::Memory {
|
||||||
// MemAvailable in MB; nullopt without /proc/meminfo.
|
// MemAvailable in MB; nullopt without /proc/meminfo.
|
||||||
std::optional<std::int64_t> Available(const std::filesystem::path& path = "/proc/meminfo");
|
std::optional<std::int64_t> Available(const std::filesystem::path& path = "/proc/meminfo");
|
||||||
// REVIEW_GATES_MEM_RESERVE_MB, else 10 % of MemTotal, at least 2048.
|
// REVIEW_GATES_MEM_RESERVE_MB, else 10 % of MemTotal, at least 2048.
|
||||||
// std::invalid_argument for a value that is no number.
|
// Dyn::Error for a value that is no number (ValueError) or is infinite
|
||||||
|
// (OverflowError); a value past int64 saturates.
|
||||||
std::int64_t Reserve(const std::optional<std::string>& configured, const std::filesystem::path& path = "/proc/meminfo");
|
std::int64_t Reserve(const std::optional<std::string>& configured, const std::filesystem::path& path = "/proc/meminfo");
|
||||||
std::int64_t Reserve();
|
std::int64_t Reserve();
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -212,10 +212,13 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"ok","total
|
||||||
json.out.substr(0, 600) + json.err);
|
json.out.substr(0, 600) + json.err);
|
||||||
const Process::Captured none = Self({"seats"}, base / "plain", withFake);
|
const Process::Captured none = Self({"seats"}, base / "plain", withFake);
|
||||||
const Process::Captured noFile = Self({"seats", "--env-file", "missing.env"}, base / "plain", withFake);
|
const Process::Captured noFile = Self({"seats", "--env-file", "missing.env"}, base / "plain", withFake);
|
||||||
Check("seats: no seat configured is exit 1 with what it looked for; a missing env file is exit 1 with the read error",
|
Write(base / "plain" / "bad.env", std::string("\xff\n", 2));
|
||||||
|
const Process::Captured badFile = Self({"seats", "--env-file", "bad.env"}, base / "plain", withFake);
|
||||||
|
Check("seats: no seat configured is exit 1 with what it looked for; a missing env file and one that is not UTF-8 are exit 1 with the read error",
|
||||||
none.returnCode == 1 && none.out.starts_with("no seat configured (CLAUDE_CODE_OAUTH_TOKEN1..6 / ANTHROPIC_API_KEY1..6; --local probes your own login)")
|
none.returnCode == 1 && none.out.starts_with("no seat configured (CLAUDE_CODE_OAUTH_TOKEN1..6 / ANTHROPIC_API_KEY1..6; --local probes your own login)")
|
||||||
&& noFile.returnCode == 1 && noFile.err.contains("FileNotFoundError: [Errno 2] No such file or directory: 'missing.env'"),
|
&& noFile.returnCode == 1 && noFile.err.contains("FileNotFoundError: [Errno 2] No such file or directory: 'missing.env'") && badFile.returnCode == 1
|
||||||
none.out + noFile.err);
|
&& badFile.err.contains("UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte"),
|
||||||
|
none.out + noFile.err + badFile.err);
|
||||||
|
|
||||||
// browser: its usage (an install fetches from npm).
|
// browser: its usage (an install fetches from npm).
|
||||||
const Process::Captured noAction = Self({"browser"}, base);
|
const Process::Captured noAction = Self({"browser"}, base);
|
||||||
|
|
|
||||||
|
|
@ -238,6 +238,14 @@ int main() {
|
||||||
&& out[0] == std::format("{} - this pass has no database (the driver gives one to a pass with the supabase environment)", PgWire::NoDatabase)
|
&& out[0] == std::format("{} - this pass has no database (the driver gives one to a pass with the supabase environment)", PgWire::NoDatabase)
|
||||||
&& out[1] == std::format("{} - the driver's SQL broker does not answer (invalid literal for int() with base 10: 'x')", PgWire::NoDatabase)
|
&& out[1] == std::format("{} - the driver's SQL broker does not answer (invalid literal for int() with base 10: 'x')", PgWire::NoDatabase)
|
||||||
&& out[2].ends_with("does not answer ([Errno 111] Connection refused)"), Show(out));
|
&& out[2].ends_with("does not answer ([Errno 111] Connection refused)"), Show(out));
|
||||||
|
std::string overflow;
|
||||||
|
try {
|
||||||
|
PgWire::CallBroker("127.0.0.1:70000#t", "x", false, collect);
|
||||||
|
} catch (const Dyn::Error& e) {
|
||||||
|
overflow = std::format("{}: {}", e.Kind(), e.what());
|
||||||
|
}
|
||||||
|
Check("call_broker: a port past 65535 is an OverflowError, which call_broker does not catch (exit 1)",
|
||||||
|
overflow == "OverflowError: getsockaddrarg: port must be 0-65535.", overflow);
|
||||||
broker.Close();
|
broker.Close();
|
||||||
out.clear();
|
out.clear();
|
||||||
Check("broker: closed, its port answers no more", PgWire::CallBroker(address, "select 1", false, collect) == 2 && out.at(0).ends_with("([Errno 111] Connection refused)"), Show(out));
|
Check("broker: closed, its port answers no more", PgWire::CallBroker(address, "select 1", false, collect) == 2 && out.at(0).ends_with("([Errno 111] Connection refused)"), Show(out));
|
||||||
|
|
|
||||||
|
|
@ -358,6 +358,12 @@ echo "{\"type\":\"result\",\"subtype\":\"success\",\"is_error\":false,\"result\"
|
||||||
dump.is_object() ? Text(dump.at("stoppedAt")) : Read(base / std::format("{}.err", name))));
|
dump.is_object() ? Text(dump.at("stoppedAt")) : Read(base / std::format("{}.err", name))));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An env file that is not UTF-8: the preflight stops the run.
|
||||||
|
Write(base / "bad.env", std::string("\xff\n", 2));
|
||||||
|
const Process::Captured badEnv = run({"--no-reuse", "--no-salvage", "--env-file", (base / "bad.env").string()});
|
||||||
|
Check("preflight: an env file that is not UTF-8 stops the run, exit 3, with the decoder's own words",
|
||||||
|
badEnv.returnCode == 3 && badEnv.err.contains("preflight: 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte"), badEnv.err);
|
||||||
|
|
||||||
// A stop between two gates: the next gate's pool starts aborted and
|
// A stop between two gates: the next gate's pool starts aborted and
|
||||||
// runs nothing of it.
|
// runs nothing of it.
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -104,6 +104,19 @@ int main() {
|
||||||
Check("memory: MemAvailable and MemTotal in MB; the reserve is 10 % of MemTotal (at least 2048 MB) unless REVIEW_GATES_MEM_RESERVE_MB says otherwise; no file, no number",
|
Check("memory: MemAvailable and MemTotal in MB; the reserve is 10 % of MemTotal (at least 2048 MB) unless REVIEW_GATES_MEM_RESERVE_MB says otherwise; no file, no number",
|
||||||
Memory::Available(mi) == 80194 && Platform::MemInfo(mi).at("MemTotal") == 128292 && Memory::Reserve(std::nullopt, mi) == 12829 && Memory::Reserve(std::string("4096"), mi) == 4096
|
Memory::Available(mi) == 80194 && Platform::MemInfo(mi).at("MemTotal") == 128292 && Memory::Reserve(std::nullopt, mi) == 12829 && Memory::Reserve(std::string("4096"), mi) == 4096
|
||||||
&& Memory::Reserve(std::nullopt, td / "none") == 2048 && !Memory::Available(td / "none"));
|
&& Memory::Reserve(std::nullopt, td / "none") == 2048 && !Memory::Available(td / "none"));
|
||||||
|
auto raised = [&](std::string_view value) {
|
||||||
|
try {
|
||||||
|
Memory::Reserve(std::string(value), mi);
|
||||||
|
return std::string("nothing");
|
||||||
|
} catch (const Dyn::Error& e) {
|
||||||
|
return std::format("{}: {}", e.Kind(), e.what());
|
||||||
|
}
|
||||||
|
};
|
||||||
|
Check("memory: a reserve that is no number raises (exit 1); one past int64 saturates; every number spelling is taken",
|
||||||
|
raised("x") == "ValueError: could not convert string to float: 'x'" && raised("nan") == "ValueError: cannot convert float NaN to integer"
|
||||||
|
&& raised("-inf") == "OverflowError: cannot convert float infinity to integer" && Memory::Reserve(std::string("1e30"), mi) == std::numeric_limits<std::int64_t>::max()
|
||||||
|
&& Memory::Reserve(std::string(" 4_096.9 "), mi) == 4096,
|
||||||
|
raised("x") + raised("nan"));
|
||||||
const std::int64_t me = getpid();
|
const std::int64_t me = getpid();
|
||||||
const std::vector<std::int64_t> tree = Platform::Tree(me, Platform::PpidMap());
|
const std::vector<std::int64_t> tree = Platform::Tree(me, Platform::PpidMap());
|
||||||
const std::vector<std::int64_t> gone = {(1 << 22) + 7};
|
const std::vector<std::int64_t> gone = {(1 << 22) + 7};
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue