corebenchrunner: Use robust error logic

Using match-statements to catch errors can be unsafe. If there are
multiple error-types, a match-statement targeted at the wrong one can
let errors fall through. A bug like this was fixed recently. This commit
attempts to guard against cases like that by simplifying the error
hierarchy, removing all types but one, and by using simpler
if-statements instead of match-statements where this makes sense.

Change-Id: I4b4307b9b94d98833cfb26ac86095f551e918470
Reviewed-by: Daniel Smith <daniel.smith@qt.io>

8 files changed, 111 insertions, 138 deletions

diff --git a/scripts/corebenchrunner/common.py b/scripts/corebenchrunner/common.py
index 112a71d7..8407b18c 100644
--- a/scripts/corebenchrunner/common.py
+++ b/scripts/corebenchrunner/common.py
@@ -21,10 +21,6 @@ class Error:
         self.message = message
 
 
-class CommandError(Error):
-    pass
-
-
 class Command:
     @staticmethod
     async def run(
@@ -32,7 +28,7 @@ class Command:
         output_file: Optional[str] = None,
         timeout: Optional[int] = None,
         cwd: Optional[str] = None,
-    ) -> Optional[CommandError]:
+    ) -> Optional[Error]:
         if output_file is None:
             process = await asyncio.create_subprocess_exec(
                 *arguments,
@@ -40,7 +36,7 @@ class Command:
             )
         elif os.path.exists(output_file):
             command = " ".join(arguments)
-            return CommandError(f'Output file of command "{command}" exists: {output_file}')
+            return Error(f'Output file of command "{command}" exists: {output_file}')
         else:
             with open(output_file, "w") as f:
                 process = await asyncio.create_subprocess_exec(
@@ -57,76 +53,72 @@ class Command:
             message = f'Command "{command}" timed out after {timeout} seconds'
             if output_file:
                 message += f"; output was redirected to {output_file}"
-            return CommandError(message)
+            return Error(message)
 
         if process.returncode != 0:
             command = " ".join(arguments)
             message = f'Command "{command} returned non-zero code {process.returncode}'
             if output_file:
                 message += f"; output was redirected to {output_file}"
-            return CommandError(message)
+            return Error(message)
         else:
             return None
 
 
-class XmlParserError(Error):
-    pass
-
-
 class XmlParser:
     def __init__(self, element: ET.Element) -> None:
         self.element = element
 
     @staticmethod
-    def load(file: str, tag: str) -> Union["XmlParser", XmlParserError]:
+    def load(file: str, tag: str) -> Union["XmlParser", Error]:
         try:
             tree = ET.parse(file)
         except ET.ParseError as error:
-            return XmlParserError(str(error))
+            return Error(str(error))
         else:
             element = tree.getroot()
             if element.tag != tag:
-                return XmlParserError(f"Root element should be {tag} but was {element.tag}")
+                return Error(f"Root element should be {tag} but was {element.tag}")
             else:
                 return XmlParser(element)
 
     def children(self, tag: str) -> List["XmlParser"]:
         return [XmlParser(element) for element in self.element.findall(tag)]
 
-    def child(self, tag: str) -> Union["XmlParser", XmlParserError]:
+    def child(self, tag: str) -> Union["XmlParser", Error]:
         children = self.children(tag)
         if len(children) == 0:
-            return XmlParserError(f"{self.element.tag} has no {tag} children")
+            return Error(f"{self.element.tag} has no {tag} children")
         elif len(children) == 1:
             return children[0]
         else:
-            return XmlParserError(f"{self.element.tag} has multiple {tag} children")
+            return Error(f"{self.element.tag} has multiple {tag} children")
 
-    def string_attribute(self, name: str) -> Union[str, XmlParserError]:
+    def string_attribute(self, name: str) -> Union[str, Error]:
         try:
             return self.element.attrib[name]
         except KeyError:
-            return XmlParserError(f"{self.element.tag} has no attribute {name}")
+            return Error(f"{self.element.tag} has no attribute {name}")
 
-    def integer_attribute(self, name: str) -> Union[int, XmlParserError]:
+    def integer_attribute(self, name: str) -> Union[int, Error]:
         string = self.string_attribute(name)
         match string:
-            case XmlParserError() as error:
+            case Error() as error:
                 return error
 
         try:
             return int(string)
         except ValueError:
-            return XmlParserError(f"Could not parse {self.element.tag}.{name} as integer: {string}")
+            return Error(f"Could not parse {self.element.tag}.{name} as integer: {string}")
 
-    def decimal_attribute(self, name: str) -> Union[decimal.Decimal, XmlParserError]:
+    def decimal_attribute(self, name: str) -> Union[decimal.Decimal, Error]:
         string = self.string_attribute(name)
         match string:
-            case XmlParserError() as error:
+            case Error() as error:
                 return error
         string = string.replace(",", ".")  # QtTest uses both, depending on the version.
 
         try:
             return decimal.Decimal(string)
         except decimal.InvalidOperation:
-            return XmlParserError(f"Could not parse {self.element.tag}.{name} as decimal: {string}")
+            return Error(f"Could not parse {self.element.tag}.{name} as decimal: {string}")
diff --git a/scripts/corebenchrunner/database.py b/scripts/corebenchrunner/database.py
index baab1c58..bf878518 100644
--- a/scripts/corebenchrunner/database.py
+++ b/scripts/corebenchrunner/database.py
@@ -7,6 +7,7 @@ from typing import Any, Dict, List, Optional, Union
 import influxdb_client  # type: ignore
 from influxdb_client.client.influxdb_client_async import InfluxDBClientAsync  # type: ignore
 
+import common
 import coordinator
 import host
 import qt
@@ -56,7 +57,7 @@ class Environment(storage.Environment):
         work_item: coordinator.WorkItem,
         host_info: host.Info,
         logger: logging.Logger,
-    ) -> Optional[storage.Error]:
+    ) -> Optional[common.Error]:
         logger.debug("Preparing results for upload")
         data_points = self.prepare_data(
             results=results,
@@ -66,7 +67,7 @@ class Environment(storage.Environment):
             logger=logger,
         )
         match data_points:
-            case storage.Error() as error:
+            case common.Error() as error:
                 return error
 
         logger.info("Uploading results")
@@ -75,7 +76,7 @@ class Environment(storage.Environment):
                 bucket=f"{self.database_name}/autogen", record=data_points
             )
         except Exception as exception:
-            return storage.Error(f"InfluxDB exception: {repr(exception)}")
+            return common.Error(f"InfluxDB exception: {repr(exception)}")
 
         return None
 
@@ -86,7 +87,7 @@ class Environment(storage.Environment):
         work_item: coordinator.WorkItem,
         host_info: host.Info,
         logger: logging.Logger,
-    ) -> Union[List[influxdb_client.Point], storage.Error]:
+    ) -> Union[List[influxdb_client.Point], common.Error]:
         """
         Prepare data for upload.
 
@@ -116,7 +117,7 @@ class Environment(storage.Environment):
         )
 
         if not benchmark_results:
-            return storage.Error("No data points in test results")
+            return common.Error("No data points in test results")
         else:
             return benchmark_run + test_file_issues + benchmark_results
 
diff --git a/scripts/corebenchrunner/git.py b/scripts/corebenchrunner/git.py
index 653c6f05..b1c1abe5 100644
--- a/scripts/corebenchrunner/git.py
+++ b/scripts/corebenchrunner/git.py
@@ -15,10 +15,6 @@ class Remote:
         self.url = url
 
 
-class Error(common.Error):
-    pass
-
-
 class Repository:
     """
     A local repository cloned from a remote repository.
@@ -32,11 +28,11 @@ class Repository:
         remote: Remote,
         parent_directory: str,
         log_directory: str,
-    ) -> Union["Repository", Error]:
+    ) -> Union["Repository", common.Error]:
         try:
             name = remote.url.rsplit("/", maxsplit=1)[1]
         except IndexError:
-            return Error("Failed to extract repository name from remote URL")
+            return common.Error("Failed to extract repository name from remote URL")
 
         directory = os.path.join(parent_directory, name)
         error = await common.Command.run(
@@ -45,12 +41,12 @@ class Repository:
             timeout=COMMAND_TIMEOUT,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error() as error:
+                return error
 
         return Repository(directory)
 
-    async def reset(self, revision: str, log_directory: str) -> Optional[Error]:
+    async def reset(self, revision: str, log_directory: str) -> Optional[common.Error]:
         error = await common.Command.run(
             arguments=["git", "fetch", "origin", revision],
             output_file=os.path.join(log_directory, "fetch.log"),
@@ -58,8 +54,8 @@ class Repository:
             cwd=self.directory,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error() as error:
+                return error
 
         error = await common.Command.run(
             arguments=["git", "clean", "-dfx"],
@@ -68,8 +64,8 @@ class Repository:
             cwd=self.directory,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error(message):
+                return common.Error(message)
 
         error = await common.Command.run(
             arguments=["git", "reset", "--hard", revision],
@@ -78,7 +74,7 @@ class Repository:
             cwd=self.directory,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error(message):
+                return common.Error(message)
 
         return None
diff --git a/scripts/corebenchrunner/host.py b/scripts/corebenchrunner/host.py
index f7c4c8fa..881a174e 100644
--- a/scripts/corebenchrunner/host.py
+++ b/scripts/corebenchrunner/host.py
@@ -8,10 +8,6 @@ from typing import Union
 import common
 
 
-class Error(common.Error):
-    pass
-
-
 class Info:
     def __init__(self, name: str, os: str, cpu: str) -> None:
         self.name = name
@@ -19,23 +15,23 @@ class Info:
         self.cpu = cpu
 
     @staticmethod
-    async def gather() -> Union["Info", Error]:
+    async def gather() -> Union["Info", common.Error]:
         name = socket.gethostname()
         os = platform.platform()
         cpu = await Info.get_cpu()
         match cpu:
-            case Error() as error:
+            case common.Error() as error:
                 return error
 
         return Info(name=name, os=os, cpu=cpu)
 
     @staticmethod
-    async def get_cpu() -> Union[str, Error]:
+    async def get_cpu() -> Union[str, common.Error]:
         with open("/proc/cpuinfo") as f:
             match = re.search(
                 pattern=r"^model name\s*: ([^ ].*)$", string=f.read(), flags=re.MULTILINE
             )
         if not match:
-            return Error("Failed to parse /proc/cpuinfo")
+            return common.Error("Failed to parse /proc/cpuinfo")
         else:
             return match[1]
diff --git a/scripts/corebenchrunner/qt.py b/scripts/corebenchrunner/qt.py
index 14e52ef7..05083e69 100644
--- a/scripts/corebenchrunner/qt.py
+++ b/scripts/corebenchrunner/qt.py
@@ -15,10 +15,6 @@ BUILD_TIMEOUT = 30 * 60
 TEST_TIMEOUT = 15 * 60
 
 
-class Error(common.Error):
-    pass
-
-
 class Module:
     def __init__(self, test_files: List["TestFile"]) -> None:
         self.test_files = test_files
@@ -26,7 +22,7 @@ class Module:
     @staticmethod
     async def configure(
         build_directory: str, repository_directory: str, log_directory: str
-    ) -> Optional[Error]:
+    ) -> Optional[common.Error]:
         output_file = os.path.join(log_directory, "configure.log")
 
         error = await common.Command.run(
@@ -42,15 +38,15 @@ class Module:
             cwd=build_directory,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error() as error:
+                return error
 
         return None
 
     @staticmethod
     async def build(
         build_directory: str, log_directory: str, test_file: Optional[str], logger: logging.Logger
-    ) -> Union["Module", Error]:
+    ) -> Union["Module", common.Error]:
         target = test_file if test_file is not None else "tests/benchmarks/install/local"
 
         error = await common.Command.run(
@@ -60,14 +56,14 @@ class Module:
             cwd=build_directory,
         )
         match error:
-            case common.CommandError(message):
-                return Error(message)
+            case common.Error() as error:
+                return error
 
         logger.debug("Searching for test files")
         directory = os.path.join(build_directory, "tests", "benchmarks")
         test_files = Module.find_test_files(directory=directory, logger=logger)
         if not test_files:
-            return Error(f"Found no test files in {directory}")
+            return common.Error(f"Found no test files in {directory}")
         else:
             return Module(test_files)
 
@@ -235,7 +231,7 @@ class ResultFileParser:
         test_file = result_file.test_file
         test_case_result = ResultFileParser.parse_file(result_file.path)
         match test_case_result:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return TestFileIssue(
                     test_file=test_file,
                     description=f"Test result file is invalid: {error.message}",
@@ -244,10 +240,10 @@ class ResultFileParser:
         return TestFileResult(test_file=test_file, test_case_result=test_case_result)
 
     @staticmethod
-    def parse_file(file: str) -> Union[TestCaseResult, common.XmlParserError]:
+    def parse_file(file: str) -> Union[TestCaseResult, common.Error]:
         element = common.XmlParser.load(file=file, tag="TestCase")
         match element:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         return ResultFileParser.parse_test_case_result(element)
@@ -255,27 +251,27 @@ class ResultFileParser:
     @staticmethod
     def parse_test_case_result(
         element: common.XmlParser,
-    ) -> Union[TestCaseResult, common.XmlParserError]:
+    ) -> Union[TestCaseResult, common.Error]:
         name = element.string_attribute("name")
         match name:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         child = element.child("Duration")
         match child:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         duration = child.decimal_attribute("msecs")
         match duration:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         test_function_results = []
         for child in element.children("TestFunction"):
             result = ResultFileParser.parse_test_function_result(child)
             match result:
-                case common.XmlParserError() as error:
+                case common.Error() as error:
                     return error
 
             test_function_results.append(result)
@@ -287,17 +283,17 @@ class ResultFileParser:
     @staticmethod
     def parse_test_function_result(
         element: common.XmlParser,
-    ) -> Union[TestFunctionResult, common.XmlParserError]:
+    ) -> Union[TestFunctionResult, common.Error]:
         name = element.string_attribute("name")
         match name:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         messages = []
         for child in element.children("Message"):
             message = ResultFileParser.parse_message(child)
             match message:
-                case common.XmlParserError() as error:
+                case common.Error() as error:
                     return error
 
             messages.append(message)
@@ -306,7 +302,7 @@ class ResultFileParser:
         for child in element.children("Incident"):
             incident = ResultFileParser.parse_incident(child)
             match incident:
-                case common.XmlParserError() as error:
+                case common.Error() as error:
                     return error
 
             incidents.append(incident)
@@ -315,7 +311,7 @@ class ResultFileParser:
         for child in element.children("BenchmarkResult"):
             benchmark_result = ResultFileParser.parse_benchmark_result(child)
             match benchmark_result:
-                case common.XmlParserError() as error:
+                case common.Error() as error:
                     return error
 
             benchmark_results.append(benchmark_result)
@@ -327,36 +323,36 @@ class ResultFileParser:
     @staticmethod
     def parse_benchmark_result(
         element: common.XmlParser,
-    ) -> Union[BenchmarkResult, common.XmlParserError]:
+    ) -> Union[BenchmarkResult, common.Error]:
         tag = element.string_attribute("tag")
         match tag:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         data_tag = tag if tag != "" else None
 
         metric = element.string_attribute("metric")
         match metric:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         iterations = element.integer_attribute("iterations")
         match iterations:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         value = element.decimal_attribute("value")
         match value:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         return BenchmarkResult(data_tag=data_tag, metric=metric, iterations=iterations, value=value)
 
     @staticmethod
-    def parse_incident(element: common.XmlParser) -> Union[Incident, common.XmlParserError]:
+    def parse_incident(element: common.XmlParser) -> Union[Incident, common.Error]:
         incident_type = element.string_attribute("type")
         match incident_type:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         children = element.children("DataTag")
@@ -365,15 +361,15 @@ class ResultFileParser:
         elif len(children) == 1:
             data_tag = children[0].element.text
         else:
-            return common.XmlParserError("Incident has multiple DataTag children")
+            return common.Error("Incident has multiple DataTag children")
 
         return Incident(incident_type=incident_type, data_tag=data_tag)
 
     @staticmethod
-    def parse_message(element: common.XmlParser) -> Union[Message, common.XmlParserError]:
+    def parse_message(element: common.XmlParser) -> Union[Message, common.Error]:
         message_type = element.string_attribute("type")
         match message_type:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         children = element.children("DataTag")
@@ -382,15 +378,15 @@ class ResultFileParser:
         elif len(children) == 1:
             data_tag = children[0].element.text
         else:
-            return common.XmlParserError("Message has multiple DataTag children")
+            return common.Error("Message has multiple DataTag children")
 
         child = element.child("Description")
         match child:
-            case common.XmlParserError() as error:
+            case common.Error() as error:
                 return error
 
         description = child.element.text
         if description is None:
-            return common.XmlParserError("Message has no text")
+            return common.Error("Message has no text")
 
         return Message(message_type=message_type, data_tag=data_tag, description=description)
diff --git a/scripts/corebenchrunner/runner.py b/scripts/corebenchrunner/runner.py
index 4ead6def..c22a5e95 100644
--- a/scripts/corebenchrunner/runner.py
+++ b/scripts/corebenchrunner/runner.py
@@ -26,12 +26,6 @@ import storage
 OUTPUT_NAME = "runner"
 
 
-class Error(common.Error):
-    """
-    An error that causes the runner to exit.
-    """
-
-
 class Arguments:
     """
     Command-line arguments that are parsed by the runner.
@@ -200,7 +194,7 @@ class Configuration:
         self.git_remote = git_remote
 
     @staticmethod
-    def load(file: str, skip_upload: bool) -> Union["Configuration", Error]:
+    def load(file: str, skip_upload: bool) -> Union["Configuration", common.Error]:
         """
         Load a configuration from file and validate it.
         """
@@ -209,7 +203,7 @@ class Configuration:
             with open(file) as f:
                 dictionary = json.load(f)
         except json.JSONDecodeError as decode_error:
-            return Error(f"Failed to load configuration file: {decode_error}")
+            return common.Error(f"Failed to load configuration file: {decode_error}")
 
         errors = []
 
@@ -240,7 +234,7 @@ class Configuration:
             errors.append("Git remote URL is empty")
 
         if errors:
-            return Error("\n\t".join(["Configuration file contains errors:"] + errors))
+            return common.Error("\n\t".join(["Configuration file contains errors:"] + errors))
         else:
             return Configuration(
                 coordinator_info=coordinator_info, storage_mode=storage_mode, git_remote=git_remote
@@ -263,28 +257,28 @@ async def main(argv: List[str]) -> int:
     try:
         error = await run(arguments=arguments, logger=logger)
     except Exception:
-        error = Error(f"Unhandled exception:\n{traceback.format_exc()}")
+        error = common.Error(f"Unhandled exception:\n{traceback.format_exc()}")
 
     match error:
-        case Error(message):
+        case common.Error(message):
             logger.critical(message)
             return 1
 
     return 0
 
 
-async def run(arguments: Arguments, logger: logging.Logger) -> Optional[Error]:
+async def run(arguments: Arguments, logger: logging.Logger) -> Optional[common.Error]:
     """
     Connect to servers and do work.
     """
-    error: Optional[Error]
+    error: Optional[common.Error]
 
     logger.info("Loading the configuration")
     configuration = Configuration.load(
         file=arguments.configuration_file, skip_upload=arguments.runner_mode.skip_upload
     )
     match configuration:
-        case Error() as error:
+        case common.Error() as error:
             return error
 
     logger.info("Creating the output directory")
@@ -295,8 +289,8 @@ async def run(arguments: Arguments, logger: logging.Logger) -> Optional[Error]:
     logger.info("Gathering host information")
     host_info = await host.Info.gather()
     match host_info:
-        case host.Error(message):
-            return Error(f"Failed to gather host information: {message}")
+        case common.Error(message):
+            return common.Error(f"Failed to gather host information: {message}")
 
     logger.info("Connecting to the work server")
     async with coordinator.Connection(
@@ -313,8 +307,8 @@ async def run(arguments: Arguments, logger: logging.Logger) -> Optional[Error]:
                 log_directory=arguments.output_directory,
             )
             match git_repository:
-                case git.Error(message):
-                    return Error(f"Failed to clone the Git repository: {message}")
+                case common.Error(message):
+                    return common.Error(f"Failed to clone the Git repository: {message}")
 
             return await run_work_items(
                 output_directory=arguments.output_directory,
@@ -337,7 +331,7 @@ async def run_work_items(
     storage_environment: storage.Environment,
     git_repository: git.Repository,
     logger: logging.Logger,
-) -> Optional[Error]:
+) -> Optional[common.Error]:
     for ordinal in itertools.count(1):
         logger.info(f"Fetching work item {ordinal}")
         work_item = await coordinator_connection.fetch_work(
@@ -386,7 +380,7 @@ async def run_work_items(
         )
 
         match result:
-            case Error() as error:
+            case common.Error() as error:
                 return error
 
             case WorkItemFailure() as failure:
@@ -418,18 +412,18 @@ async def run_work_item(
     storage_environment: storage.Environment,
     git_repository: git.Repository,
     logger: logging.Logger,
-) -> Union[WorkItemFailure, Error, None]:
+) -> Union[WorkItemFailure, common.Error, None]:
     message = "Resetting the QtBase repository"
     logger.info(message)
     await coordinator_connection.send_status(
         status="git", message=message, work_item=work_item, logger=logger
     )
-    git_error = await git_repository.reset(
+    error = await git_repository.reset(
         revision=work_item.revision,
         log_directory=work_item_directory,
     )
-    match git_error:
-        case git.Error(message):
+    match error:
+        case common.Error(message):
             return WorkItemFailure(f"Error resetting the Git repository: {message}")
 
     message = "Configuring the QtBase module"
@@ -437,13 +431,13 @@ async def run_work_item(
     await coordinator_connection.send_status(
         status="configure", message=message, work_item=work_item, logger=logger
     )
-    module_error = await qt.Module.configure(
+    error = await qt.Module.configure(
         build_directory=build_directory,
         repository_directory=git_repository.directory,
         log_directory=work_item_directory,
     )
-    match module_error:
-        case qt.Error(message):
+    match error:
+        case common.Error(message):
             return WorkItemFailure(f"Error configuring QtBase: {message}")
 
     message = "Building the QtBase module"
@@ -458,7 +452,7 @@ async def run_work_item(
         logger=logger,
     )
     match module:
-        case qt.Error(message):
+        case common.Error(message):
             return WorkItemFailure(f"Error building QtBase: {message}")
 
     if runner_mode.skip_tuning:
@@ -472,10 +466,10 @@ async def run_work_item(
         )
     else:
         logger.info("Tuning performance to reduce system noise")
-        command_error = await common.Command.run(["sudo", "prep_bench"])
-        match command_error:
-            case common.CommandError(message):
-                return Error(f"Failed to tune performance: {message}")
+        error = await common.Command.run(["sudo", "prep_bench"])
+        match error:
+            case common.Error(message):
+                return common.Error(f"Failed to tune performance: {message}")
 
         try:
             result_files, run_issues = await run_test_files(
@@ -487,11 +481,11 @@ async def run_work_item(
                 logger=logger,
             )
         finally:
-            command_error = await common.Command.run(["sudo", "unprep_bench"])
+            error = await common.Command.run(["sudo", "unprep_bench"])
 
-        match command_error:
-            case common.CommandError(message):
-                return Error(f"Failed to revert performance tuning: {message}")
+        match error:
+            case common.Error(message):
+                return common.Error(f"Failed to revert performance tuning: {message}")
 
     results, parse_issues = parse_results(result_files=result_files, logger=logger)
 
@@ -506,7 +500,7 @@ async def run_work_item(
         logger=logger,
     )
     match error:
-        case storage.Error(message):
+        case common.Error(message):
             return WorkItemFailure(f"Error storing results: {message}")
 
     return None
@@ -605,7 +599,7 @@ def create_logger(verbose: bool) -> logging.Logger:
     return logger
 
 
-def create_output_directory(path: str, overwrite: bool) -> Optional[Error]:
+def create_output_directory(path: str, overwrite: bool) -> Optional[common.Error]:
     if not os.path.exists(path):
         os.mkdir(path)
         return None
@@ -614,7 +608,7 @@ def create_output_directory(path: str, overwrite: bool) -> Optional[Error]:
         os.mkdir(path)
         return None
     else:
-        return Error("Output directory exists (use --overwrite to remove it)")
+        return common.Error("Output directory exists (use --overwrite to remove it)")
 
 
 if __name__ == "__main__":
diff --git a/scripts/corebenchrunner/storage.py b/scripts/corebenchrunner/storage.py
index 69150184..480cdbc4 100644
--- a/scripts/corebenchrunner/storage.py
+++ b/scripts/corebenchrunner/storage.py
@@ -9,10 +9,6 @@ import host
 import qt
 
 
-class Error(common.Error):
-    pass
-
-
 class Mode:
     """
     Decides how the runner should store results.
@@ -42,7 +38,7 @@ class Environment:
         work_item: coordinator.WorkItem,
         host_info: host.Info,
         logger: logging.Logger,
-    ) -> Optional[Error]:
+    ) -> Optional[common.Error]:
         raise NotImplementedError()  # Should be overridden by subclasses.
 
 
@@ -73,6 +69,6 @@ class DropEnvironment(Environment):
         work_item: coordinator.WorkItem,
         host_info: host.Info,
         logger: logging.Logger,
-    ) -> Optional[Error]:
+    ) -> Optional[common.Error]:
         logger.warning("Dropping results")
         return None
diff --git a/scripts/corebenchrunner/tests/test_runner.py b/scripts/corebenchrunner/tests/test_runner.py
index 0ece84ca..d4de0190 100644
--- a/scripts/corebenchrunner/tests/test_runner.py
+++ b/scripts/corebenchrunner/tests/test_runner.py
@@ -5,6 +5,7 @@ import tempfile
 import unittest
 from typing import cast
 
+import common
 import runner
 
 
@@ -26,6 +27,7 @@ class TestConfiguration(unittest.TestCase):
         with tempfile.NamedTemporaryFile(mode="w") as f:
             json.dump(
                 {
+                    # Empty fields are errors.
                     "coordinator_info": {"url": "https://coordinator.com/", "secret": ""},
                     "qtbase_git_remote": {"url": "ssh://codereview.qt-project.org/qt/qtbase"},
                 },
@@ -34,6 +36,6 @@ class TestConfiguration(unittest.TestCase):
             f.seek(0)
             configuration = runner.Configuration.load(file=f.name, skip_upload=True)
 
-        self.assertIsInstance(configuration, runner.Error)
-        error = cast(runner.Error, configuration)
+        self.assertIsInstance(configuration, common.Error)
+        error = cast(common.Error, configuration)
         self.assertEqual(error.message.splitlines()[0], "Configuration file contains errors:")