Improve error messaging around conversion errors

graphql-dotnet · May 10, 2017 · 1f7471c · 1f7471c
1 parent 07e0c99
commit 1f7471c
Show file tree

Hide file tree

Showing 13 changed files with 60 additions and 20 deletions.
diff --git a/src/GraphQL.Conventions/Adapters/ResolutionContext.cs b/src/GraphQL.Conventions/Adapters/ResolutionContext.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Threading;
 using GraphQL.Conventions.Types.Descriptors;
@@ -31,6 +32,16 @@ public object GetArgument(string name, object defaultValue = null)
             }
         }
 
+        public object GetArgument(GraphArgumentInfo argument)
+        {
+            var value = GetArgument(argument.Name, argument.DefaultValue);
+            if (!argument.Type.IsNullable && value == null)
+            {
+                throw new ArgumentException($"Null value provided for non-nullable argument '{argument.Name}'.");
+            }
+            return value;
+        }
+
         public void SetArgument(string name, object value)
         {
             lock (_lock)

diff --git a/src/GraphQL.Conventions/Adapters/Resolvers/FieldResolver.cs b/src/GraphQL.Conventions/Adapters/Resolvers/FieldResolver.cs
@@ -56,7 +56,7 @@ private object CallMethod(GraphFieldInfo fieldInfo, IResolutionContext context)
 
             var arguments = fieldInfo
                 .Arguments
-                .Select(arg => context.GetArgument(arg.Name, arg.DefaultValue));
+                .Select(arg => context.GetArgument(arg));
 
             return methodInfo?.Invoke(source, arguments.ToArray());
         }

diff --git a/src/GraphQL.Conventions/Adapters/Resolvers/WrappedAsyncFieldResolver.cs b/src/GraphQL.Conventions/Adapters/Resolvers/WrappedAsyncFieldResolver.cs
@@ -60,8 +60,7 @@ private object CallMethod(GraphFieldInfo fieldInfo, IResolutionContext context)
 
             var arguments = fieldInfo
                 .Arguments
-                .Select(arg => context.GetArgument(arg.Name, arg.DefaultValue));
-
+                .Select(arg => context.GetArgument(arg));
 
             if (fieldInfo.Type.IsTask)
             {

diff --git a/src/GraphQL.Conventions/Adapters/Resolvers/WrappedSyncFieldResolver.cs b/src/GraphQL.Conventions/Adapters/Resolvers/WrappedSyncFieldResolver.cs
@@ -59,8 +59,7 @@ private object CallMethod(GraphFieldInfo fieldInfo, IResolutionContext context)
 
             var arguments = fieldInfo
                 .Arguments
-                .Select(arg => context.GetArgument(arg.Name, arg.DefaultValue));
-
+                .Select(arg => context.GetArgument(arg));
 
             if (fieldInfo.Type.IsTask)
             {

diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/CollectionWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/CollectionWrapper.cs
@@ -13,7 +13,7 @@ public CollectionWrapper(IWrapper parent)
         {
         }
 
-        public override object WrapValue(GraphTypeInfo typeInfo, object value)
+        public override object WrapValue(GraphEntityInfo entityInfo, GraphTypeInfo typeInfo, object value)
         {
             if (typeInfo.IsListType)
             {
@@ -27,7 +27,7 @@ public override object WrapValue(GraphTypeInfo typeInfo, object value)
                 var list = Activator.CreateInstance(listType) as IList;
                 foreach (var obj in input)
                 {
-                    list.Add(_parent.Wrap(typeInfo.TypeParameter, obj));
+                    list.Add(_parent.Wrap(entityInfo, typeInfo.TypeParameter, obj));
                 }
                 return typeInfo.IsArrayType
                     ? list.ConvertToArrayRuntime(elementType)

diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/IWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/IWrapper.cs
@@ -4,6 +4,6 @@ namespace GraphQL.Conventions.Attributes.Execution.Wrappers
 {
     public interface IWrapper
     {
-        object Wrap(GraphTypeInfo type, object value);
+        object Wrap(GraphEntityInfo entity, GraphTypeInfo type, object value);
     }
 }
diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/NonNullWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/NonNullWrapper.cs
@@ -1,13 +1,18 @@
+using System;
 using GraphQL.Conventions.Types.Descriptors;
 
 namespace GraphQL.Conventions.Attributes.Execution.Wrappers
 {
     public class NonNullWrapper : WrapperBase
     {
-        public override object WrapValue(GraphTypeInfo typeInfo, object value)
+        public override object WrapValue(GraphEntityInfo entityInfo, GraphTypeInfo typeInfo, object value)
         {
             if (!typeInfo.IsNullable)
             {
+                if (value == null)
+                {
+                    throw new ArgumentException($"Null value provided for non-nullable {GetEntityDescription(entityInfo)} '{entityInfo.Name}'.");
+                }
                 return NonNull.Construct(typeInfo, value);
             }
             return value;

diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/ObjectWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/ObjectWrapper.cs
@@ -14,7 +14,7 @@ public ObjectWrapper(IWrapper parent)
         {
         }
 
-        public override object WrapValue(GraphTypeInfo typeInfo, object value)
+        public override object WrapValue(GraphEntityInfo entityInfo, GraphTypeInfo typeInfo, object value)
         {
             var input = value as Dictionary<string, object>;
             if (typeInfo.IsInputType && input != null)
@@ -36,7 +36,7 @@ public override object WrapValue(GraphTypeInfo typeInfo, object value)
                     {
                         throw new Exception($"Invalid field '{field.Name}' on input object; must be a property.");
                     }
-                    propertyInfo.SetValue(obj, _parent.Wrap(field.Type, fieldValue));
+                    propertyInfo.SetValue(obj, _parent.Wrap(field, field.Type, fieldValue));
                 }
                 return obj;
             }

diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/PrimitiveWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/PrimitiveWrapper.cs
@@ -6,13 +6,20 @@ namespace GraphQL.Conventions.Attributes.Execution.Wrappers
 {
     public class PrimitiveWrapper : WrapperBase
     {
-        public override object WrapValue(GraphTypeInfo typeInfo, object value)
+        public override object WrapValue(GraphEntityInfo entityInfo, GraphTypeInfo typeInfo, object value)
         {
             if (typeInfo.IsPrimitive &&
                 !typeInfo.IsEnumerationType &&
                 value is IConvertible)
             {
-                return Convert.ChangeType(value, typeInfo.GetTypeRepresentation().AsType());
+                try
+                {
+                    return Convert.ChangeType(value, typeInfo.GetTypeRepresentation().AsType());
+                }
+                catch (Exception ex)
+                {
+                    throw new ArgumentException($"Unable to cast {GetEntityDescription(entityInfo)} '{entityInfo.Name}' to '{typeInfo.Name}'.", ex);
+                }
             }
             return value;
         }

diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/ValueWrapper.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/ValueWrapper.cs
@@ -12,6 +12,6 @@ public ValueWrapper()
                 .Next(new PrimitiveWrapper());
         }
 
-        public override object WrapValue(GraphTypeInfo type, object value) => value;
+        public override object WrapValue(GraphEntityInfo entity, GraphTypeInfo type, object value) => value;
     }
 }
diff --git a/src/GraphQL.Conventions/Attributes/Execution/Wrappers/WrapperBase.cs b/src/GraphQL.Conventions/Attributes/Execution/Wrappers/WrapperBase.cs
@@ -22,14 +22,31 @@ public WrapperBase Next(WrapperBase unwrapper)
             NextWrapper = unwrapper;
             return NextWrapper;
         }
-        public object Wrap(GraphTypeInfo type, object value)
+        public object Wrap(GraphEntityInfo entity, GraphTypeInfo type, object value)
         {
-            value = WrapValue(type, value);
+            value = WrapValue(entity, type, value);
             return NextWrapper != null
-                ? NextWrapper.Wrap(type, value)
+                ? NextWrapper.Wrap(entity, type, value)
                 : value;
         }
 
-        public abstract object WrapValue(GraphTypeInfo type, object value);
+        public abstract object WrapValue(GraphEntityInfo entity, GraphTypeInfo type, object value);
+
+        protected string GetEntityDescription(GraphEntityInfo entity)
+        {
+            if (entity is GraphTypeInfo)
+            {
+                return "type";
+            }
+            if (entity is GraphFieldInfo)
+            {
+                return "field";
+            }
+            if (entity is GraphArgumentInfo)
+            {
+                return "argument";
+            }
+            return "entity";
+        }
     }
 }
diff --git a/src/GraphQL.Conventions/Execution/IResolutionContext.cs b/src/GraphQL.Conventions/Execution/IResolutionContext.cs
@@ -9,6 +9,8 @@ public interface IResolutionContext
 
         object GetArgument(string name, object defaultValue = null);
 
+        object GetArgument(GraphArgumentInfo argument);
+
         void SetArgument(string name, object value);
 
         object RootValue { get; }

diff --git a/src/GraphQL.Conventions/Handlers/ExecutionFilterAttributeHandler.cs b/src/GraphQL.Conventions/Handlers/ExecutionFilterAttributeHandler.cs
@@ -65,8 +65,8 @@ private void ResolveArgument(GraphArgumentInfo argument, IResolutionContext reso
             }
             else
             {
-                var argumentValue = resolutionContext.GetArgument(argument.Name, argument.DefaultValue);
-                resolutionContext.SetArgument(argument.Name, Wrapper.Wrap(argument.Type, argumentValue));
+                var argumentValue = resolutionContext.GetArgument(argument);
+                resolutionContext.SetArgument(argument.Name, Wrapper.Wrap(argument, argument.Type, argumentValue));
             }
         }
     }