Re: [PATCH v2 1/8] qapi/error: Repurpose QAPIError as a generic exception base class

[John Snow]

On 4/15/21 2:44 AM, Markus Armbruster wrote:
John Snow <js...@redhat.com> writes:
Rename QAPIError to QAPISourceError, and then create a new QAPIError
class that serves as the basis for all of our other custom exceptions.

Isn't the existing QAPIError such a base class already?  Peeking
ahead...  aha, your new base class is abstract.  Can you explain why we
that's useful?


Sure. The existing QAPIError class assumes that it's an error that has a 
source location, but not all errors will. The idea is that an abstract 
error class can be used as the ancestor for all other errors in a 
type-safe way, such that:

try:
    qapi.something_or_other()
except QAPIError as err:
    err.some_sort_of_method()

(1) This is a type-safe construct, and
(2) This is sufficient to catch all errors that originate from within 
the library, regardless of their exact type.

Primarily, it's a ploy to get the SourceInfo stuff out of the 
common/root exception for type safety reasons.

(Later in the series, you'll see there's a few places where we try to 
fake SourceInfo stuff in order to use QAPIError, by shuffling this 
around, we allow ourselves to raise exceptions that don't need this 
hackery.)

Add docstrings to explain the intended function of each error class.

Signed-off-by: John Snow <js...@redhat.com>
---
  docs/sphinx/qapidoc.py |  3 ++-
  scripts/qapi/error.py  | 11 +++++++++--
  scripts/qapi/schema.py |  5 +++--
  3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index b7b86b5dff..458b1f477e 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -34,7 +34,8 @@
  from sphinx.util.nodes import nested_parse_with_titles
  import sphinx
  from qapi.gen import QAPISchemaVisitor
-from qapi.schema import QAPIError, QAPISemError, QAPISchema
+from qapi.error import QAPIError, QAPISemError
+from qapi.schema import QAPISchema
  
  
  # Sphinx up to 1.6 uses AutodocReporter; 1.7 and later
diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
index ae60d9e2fe..126dda7c9b 100644
--- a/scripts/qapi/error.py
+++ b/scripts/qapi/error.py
@@ -13,6 +13,11 @@
  
  
  class QAPIError(Exception):
+    """Base class for all exceptions from the QAPI package."""
+
+
+class QAPISourceError(QAPIError):
+    """Error class for all exceptions identifying a source location."""
      def __init__(self, info, col, msg):
          Exception.__init__(self)
          self.info = info
@@ -27,7 +32,8 @@ def __str__(self):
          return loc + ': ' + self.msg
  
  
-class QAPIParseError(QAPIError):
+class QAPIParseError(QAPISourceError):
+    """Error class for all QAPI schema parsing errors."""
      def __init__(self, parser, msg):
          col = 1
          for ch in parser.src[parser.line_pos:parser.pos]:
@@ -38,6 +44,7 @@ def __init__(self, parser, msg):
          super().__init__(parser.info, col, msg)
  
  
-class QAPISemError(QAPIError):
+class QAPISemError(QAPISourceError):
+    """Error class for semantic QAPI errors."""
      def __init__(self, info, msg):
          super().__init__(info, None, msg)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 01cdd753cd..1849c3e45f 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -20,7 +20,7 @@
  from typing import Optional
  
  from .common import POINTER_SUFFIX, c_name
-from .error import QAPIError, QAPISemError
+from .error import QAPISemError, QAPISourceError
  from .expr import check_exprs
  from .parser import QAPISchemaParser
  
@@ -878,7 +878,8 @@ def _def_entity(self, ent):
          other_ent = self._entity_dict.get(ent.name)
          if other_ent:
              if other_ent.info:
-                where = QAPIError(other_ent.info, None, "previous definition")
+                where = QAPISourceError(other_ent.info, None,
+                                        "previous definition")
                  raise QAPISemError(
                      ent.info,
                      "'%s' is already defined\n%s" % (ent.name, where))