Merge pull request #9737 from obsidiansystems/sys-error-split

Separate `SystemError` from `SysError`

src/libcmd/repl.cc

@@ -254,7 +254,7 @@ void NixRepl::mainLoop()
     rl_readline_name = "nix-repl";
     try {
         createDirs(dirOf(historyFile));
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         logWarning(e.info());
     }
 #ifndef USE_READLINE

src/libstore/build/local-derivation-goal.cc

@@ -1495,7 +1495,7 @@ void LocalDerivationGoal::startDaemon()
                     daemon::processConnection(store, from, to,
                         NotTrusted, daemon::Recursive);
                     debug("terminated daemon connection");
-                } catch (SysError &) {
+                } catch (SystemError &) {
                     ignoreException();
                 }
             });
@@ -1707,7 +1707,7 @@ void LocalDerivationGoal::runChild()
         try {
             if (drv->isBuiltin() && drv->builder == "builtin:fetchurl")
                 netrcData = readFile(settings.netrcFile);
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
 
 #if __linux__
         if (useChroot) {

src/libstore/gc.cc

@@ -413,7 +413,7 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
                     auto env_end = std::sregex_iterator{};
                     for (auto i = std::sregex_iterator{envString.begin(), envString.end(), storePathRegex}; i != env_end; ++i)
                         unchecked[i->str()].emplace(envFile);
-                } catch (SysError & e) {
+                } catch (SystemError & e) {
                     if (errno == ENOENT || errno == EACCES || errno == ESRCH)
                         continue;
                     throw;

src/libstore/globals.cc

@@ -118,7 +118,7 @@ void loadConfFile()
         try {
             std::string contents = readFile(path);
             globalConfig.applyConfig(contents, path);
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     };
 
     applyConfigFile(settings.nixConfDir + "/nix.conf");

src/libstore/keys.cc

@@ -19,7 +19,7 @@ PublicKeys getDefaultPublicKeys()
         try {
             SecretKey secretKey(readFile(secretKeyFile));
             publicKeys.emplace(secretKey.name, secretKey.toPublicKey());
-        } catch (SysError & e) {
+        } catch (SystemError & e) {
             /* Ignore unreadable key files. That's normal in a
                multi-user installation. */
         }

src/libstore/local-store.cc

@@ -276,7 +276,7 @@ LocalStore::LocalStore(const Params & params)
                 [[gnu::unused]] auto res2 = ftruncate(fd.get(), settings.reservedSize);
             }
         }
-    } catch (SysError & e) { /* don't care about errors */
+    } catch (SystemError & e) { /* don't care about errors */
     }
 
     /* Acquire the big fat lock in shared mode to make sure that no

src/libstore/optimise-store.cc

@@ -242,7 +242,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     /* Atomically replace the old file with the new hard link. */
     try {
         renameFile(tempLink, path);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         if (unlink(tempLink.c_str()) == -1)
             printError("unable to unlink '%1%'", tempLink);
         if (errno == EMLINK) {

src/libstore/remote-fs-accessor.cc

@@ -87,13 +87,13 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
             nars.emplace(storePath.hashPart(), narAccessor);
             return {narAccessor, restPath};
 
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
 
         try {
             auto narAccessor = makeNarAccessor(nix::readFile(cacheFile));
             nars.emplace(storePath.hashPart(), narAccessor);
             return {narAccessor, restPath};
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
 
     StringSink sink;

src/libutil/args.cc

@@ -304,7 +304,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang)
                 for (auto pos = savedArgs.begin(); pos != savedArgs.end();pos++)
                     cmdline.push_back(*pos);
             }
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
     for (auto pos = cmdline.begin(); pos != cmdline.end(); ) {
 

src/libutil/cgroup.cc

@@ -95,7 +95,7 @@ static CgroupStats destroyCgroup(const Path & cgroup, bool returnStats)
                     using namespace std::string_literals;
                     warn("killing stray builder process %d (%s)...",
                         pid, trim(replaceStrings(cmdline, "\0"s, " ")));
-                } catch (SysError &) {
+                } catch (SystemError &) {
                 }
             }
             // FIXME: pid wraparound

src/libutil/config.cc

@@ -124,7 +124,7 @@ static void applyConfigInner(const std::string & contents, const std::string & p
                 try {
                     std::string includedContents = readFile(path);
                     applyConfigInner(includedContents, p, parsedContents);
-                } catch (SysError &) {
+                } catch (SystemError &) {
                     // TODO: Do we actually want to ignore this? Or is it better to fail?
                 }
             } else if (!ignoreMissing) {

src/libutil/error.hh

@@ -178,20 +178,50 @@ MakeError(Error, BaseError);
 MakeError(UsageError, Error);
 MakeError(UnimplementedError, Error);
 
-class SysError : public Error
+/**
+ * To use in catch-blocks.
+ */
+MakeError(SystemError, Error);
+
+/**
+ * POSIX system error, created using `errno`, `strerror` friends.
+ *
+ * Throw this, but prefer not to catch this, and catch `SystemError`
+ * instead. This allows implementations to freely switch between this
+ * and `WinError` without breaking catch blocks.
+ *
+ * However, it is permissible to catch this and rethrow so long as
+ * certain conditions are not met (e.g. to catch only if `errNo =
+ * EFooBar`). In that case, try to also catch the equivalent `WinError`
+ * code.
+ *
+ * @todo Rename this to `PosixError` or similar. At this point Windows
+ * support is too WIP to justify the code churn, but if it is finished
+ * then a better identifier becomes moe worth it.
+ */
+class SysError : public SystemError
 {
 public:
     int errNo;
 
+    /**
+     * Construct using the explicitly-provided error number. `strerror`
+     * will be used to try to add additional information to the message.
+     */
     template<typename... Args>
-    SysError(int errNo_, const Args & ... args)
+    SysError(int errNo, const Args & ... args)
-        : Error("")
+        : SystemError(""), errNo(errNo)
     {
-        errNo = errNo_;
         auto hf = hintfmt(args...);
         err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo));
     }
 
+    /**
+     * Construct using the ambient `errno`.
+     *
+     * Be sure to not perform another `errno`-modifying operation before
+     * calling this constructor!
+     */
     template<typename... Args>
     SysError(const Args & ... args)
         : SysError(errno, args ...)
@@ -199,7 +229,9 @@ public:
     }
 };
 
-/** Throw an exception for the purpose of checking that exception handling works; see 'initLibUtil()'.
+/**
+ * Throw an exception for the purpose of checking that exception
+ * handling works; see 'initLibUtil()'.
  */
 void throwExceptionSelfCheck();
 

src/libutil/file-descriptor.cc

@@ -231,7 +231,7 @@ void closeMostFDs(const std::set<int> & exceptions)
             }
         }
         return;
-    } catch (SysError &) {
+    } catch (SystemError &) {
     }
 #endif
 

src/libutil/logging.cc

@@ -116,7 +116,7 @@ void writeToStderr(std::string_view s)
 {
     try {
         writeFull(STDERR_FILENO, s, false);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         /* Ignore failing writes to stderr.  We need to ignore write
            errors to ensure that cleanup code that logs to stderr runs
            to completion if the other side of stderr has been closed

src/libutil/serialise.cc

@@ -53,7 +53,7 @@ void FdSink::writeUnbuffered(std::string_view data)
     written += data.size();
     try {
         writeFull(fd, data);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         _good = false;
         throw;
     }

src/libutil/util.cc

@@ -20,7 +20,7 @@ void initLibUtil() {
     // When exception handling fails, the message tends to be printed by the
     // C++ runtime, followed by an abort.
     // For example on macOS we might see an error such as
-    // libc++abi: terminating with uncaught exception of type nix::SysError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
+    // libc++abi: terminating with uncaught exception of type nix::SystemError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
     bool caught = false;
     try {
         throwExceptionSelfCheck();

src/nix-build/nix-build.cc

@@ -148,7 +148,7 @@ static void main_nix_build(int argc, char * * argv)
                             args.push_back(word);
                 }
             }
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
 
     struct MyArgs : LegacyArgs, MixEvalArgs

src/nix/config-check.cc

@@ -107,7 +107,7 @@ struct CmdConfigCheck : StoreCommand
                     if (profileDir.find("/profiles/") == std::string::npos)
                         dirs.insert(dir);
                 }
-            } catch (SysError &) {}
+            } catch (SystemError &) {}
         }
 
         if (!dirs.empty()) {

tests/unit/libutil/logging.cc

@@ -73,7 +73,7 @@ namespace nix {
 
     }
 
-    TEST(logEI, picksUpSysErrorExitCode) {
+    TEST(logEI, picksUpSystemErrorExitCode) {
 
         MakeError(TestError, Error);
         ErrorInfo::programName = std::optional("error-unit-test");
@@ -81,12 +81,12 @@ namespace nix {
         try {
             auto x = readFile(-1);
         }
-        catch (SysError &e) {
+        catch (SystemError &e) {
             testing::internal::CaptureStderr();
             logError(e.info());
             auto str = testing::internal::GetCapturedStderr();
 
-            ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
+            ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SystemError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
         }
     }