Don't suppress SQLAlchemy errors when mapping classes

Author: Connor Brinton

graphene_sqlalchemy/tests/test_types.py

@@ -4,6 +4,7 @@
 import six
 from promise import Promise
 
+from .. import utils
 from ..registry import Registry
 from ..types import SQLAlchemyObjectType, SQLAlchemyObjectTypeOptions
 from .models import Article, Reporter
@@ -181,3 +182,35 @@ class Meta:
         resolver, TestConnection, ReporterWithCustomOptions, None, None
     )
     assert result is not None
+
+
+def test_errors_propagated():
+    # Get current `class_mapper` value
+    old_class_mapper = utils.class_mapper
+
+    try:
+        # Define unique error to detect
+        class UniqueError(Exception):
+            pass
+
+        # Redefine `class_mapper` in utils
+        def new_class_mapper(*args, **kwargs):
+            raise UniqueError()
+        utils.class_mapper = new_class_mapper
+
+        # Make sure that errors are propagated from class_mapper when instantiating classes
+        error = None
+        try:
+            class Tree(SQLAlchemyObjectType):
+                class Meta(object):
+                    model = Article
+
+        except UniqueError as e:
+            error = e
+
+        # Check that an error occured, and that it was a SQLAlchemy error
+        assert error is not None
+        assert isinstance(error, UniqueError)
+    finally:
+        # Restore original class mapper
+        utils.class_mapper = old_class_mapper

graphene_sqlalchemy/types.py

@@ -103,9 +103,11 @@ def __init_subclass_with_meta__(
         _meta=None,
         **options
     ):
-        assert is_mapped_class(model), (
-            "You need to pass a valid SQLAlchemy Model in " '{}.Meta, received "{}".'
-        ).format(cls.__name__, model)
+        # Make sure model is a valid SQLAlchemy model
+        if not is_mapped_class(model):
+            raise ValueError(
+                "You need to pass a valid SQLAlchemy Model in " '{}.Meta, received "{}".'.format(cls.__name__, model)
+            )
 
         if not registry:
             registry = get_global_registry()

graphene_sqlalchemy/utils.py

@@ -25,8 +25,17 @@ def get_query(model, context):
 def is_mapped_class(cls):
     try:
         class_mapper(cls)
-    except (ArgumentError, UnmappedClassError):
-        return False
+    except (ArgumentError, UnmappedClassError) as error:
+        # Only handle ArgumentErrors for non-class objects
+        if isinstance(error, ArgumentError) and "Class object expected" in str(error):
+            return False
+
+        # Unmapped classes return false
+        if isinstance(error, UnmappedClassError):
+            return False
+
+        # We don't know how to handle this type of error, reraise
+        raise error
     else:
         return True