Normalize deserialization errors for bad data

This normalizes all the cases of trying to set null to non-null things and giving a better exception when it happens. Will write it up better in PR.

Related to:
- #1421
- #1440

Dapper.Tests/MiscTests.cs

@@ -167,6 +167,89 @@ public void Test_Single_First_Default()
             Assert.Equal("Sequence contains more than one element", ex.Message);
         }
 
+        /// <summary>
+        /// This test is ensuring our "single row" methods also behave like a type being deserialized
+        /// and give a useful error message when the types don't match.
+        /// </summary>
+        [Fact]
+        public async Task TestConversionExceptionMessages()
+        {
+            const string sql = "Select Null;";
+
+            // Nullable is expected to work if we get a null in all cases
+            // List paths
+            var list = connection.Query<int?>(sql);
+            Assert.Null(Assert.Single(list));
+            list = await connection.QueryAsync<int?>(sql);
+            Assert.Null(Assert.Single(list));
+
+            // Single row paths
+            Assert.Null(connection.QueryFirst<int?>(sql));
+            Assert.Null(connection.QueryFirstOrDefault<int?>(sql));
+            Assert.Null(connection.QuerySingle<int?>(sql));
+            Assert.Null(connection.QuerySingleOrDefault<int?>(sql));
+
+            Assert.Null(await connection.QueryFirstAsync<int?>(sql));
+            Assert.Null(await connection.QueryFirstOrDefaultAsync<int?>(sql));
+            Assert.Null(await connection.QuerySingleAsync<int?>(sql));
+            Assert.Null(await connection.QuerySingleOrDefaultAsync<int?>(sql));
+
+            static async Task TestExceptionsAsync<T>(DbConnection connection, string sql, string exception)
+            {
+                var ex = Assert.Throws<DataException>(() => connection.Query<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = Assert.Throws<DataException>(() => connection.QueryFirst<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = Assert.Throws<DataException>(() => connection.QueryFirstOrDefault<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = Assert.Throws<DataException>(() => connection.QuerySingle<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = Assert.Throws<DataException>(() => connection.QuerySingleOrDefault<T>(sql));
+                Assert.Equal(exception, ex.Message);
+
+                ex = await Assert.ThrowsAsync<DataException>(() => connection.QueryAsync<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = await Assert.ThrowsAsync<DataException>(() => connection.QueryFirstAsync<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = await Assert.ThrowsAsync<DataException>(() => connection.QueryFirstOrDefaultAsync<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = await Assert.ThrowsAsync<DataException>(() => connection.QuerySingleAsync<T>(sql));
+                Assert.Equal(exception, ex.Message);
+                ex = await Assert.ThrowsAsync<DataException>(() => connection.QuerySingleOrDefaultAsync<T>(sql));
+                Assert.Equal(exception, ex.Message);
+            }
+
+            // Null value throws
+            await TestExceptionsAsync<int>(
+                connection,
+                "Select null as Foo",
+                "Error parsing column 0 (Foo=<null>)");
+            // Incompatible value throws (testing unnamed column bits here too)
+            await TestExceptionsAsync<int>(
+                connection,
+                "Select 'bar'",
+                "Error parsing column 0 ((Unnamed Column)=bar - String)");
+            // Null with a full type (testing position too)
+            await TestExceptionsAsync<NullTestType>(
+                connection,
+                "Select 1 Id, 'bar' Foo",
+                "Error parsing column 1 (Foo=bar - String)");
+
+            // And a ValueTuple! (testing position too)
+            // Needs love, because we handle ValueTuple differently today
+            // It'll yield a raw: typeof(System.FormatException): Input string was not in a correct format.
+            //await TestExceptionsAsync<(int Id, int Foo)>(
+            //    connection,
+            //    "Select 1 Id, 'bar' Foo",
+            //    "Error parsing column 1 (Foo=bar - String)");
+        }
+
+        private class NullTestType
+        {
+            public int Id { get; }
+            public int Foo { get; }
+        }
+
         [Fact]
         public void TestStrings()
         {

Dapper/SqlMapper.Async.cs

@@ -437,14 +437,7 @@ private static async Task<IEnumerable<T>> QueryAsync<T>(this IDbConnection cnn,
                         while (await reader.ReadAsync(cancel).ConfigureAwait(false))
                         {
                             object val = func(reader);
-                            if (val == null || val is T)
-                            {
-                                buffer.Add((T)val);
-                            }
-                            else
-                            {
-                                buffer.Add((T)Convert.ChangeType(val, convertToType, CultureInfo.InvariantCulture));
-                            }
+                            buffer.Add(GetValue<T>(reader, effectiveType, val));
                         }
                         while (await reader.NextResultAsync(cancel).ConfigureAwait(false)) { /* ignore subsequent result sets */ }
                         command.OnCompleted();
@@ -484,29 +477,11 @@ private static async Task<T> QueryRowAsync<T>(this IDbConnection cnn, Row row, T
                     ? CommandBehavior.SequentialAccess | CommandBehavior.SingleResult // need to allow multiple rows, to check fail condition
                     : CommandBehavior.SequentialAccess | CommandBehavior.SingleResult | CommandBehavior.SingleRow, cancel).ConfigureAwait(false);
 
-                    T result = default(T);
+                    T result = default;
                     if (await reader.ReadAsync(cancel).ConfigureAwait(false) && reader.FieldCount != 0)
                     {
-                        var tuple = info.Deserializer;
-                        int hash = GetColumnHash(reader);
-                        if (tuple.Func == null || tuple.Hash != hash)
-                        {
-                            tuple = info.Deserializer = new DeserializerState(hash, GetDeserializer(effectiveType, reader, 0, -1, false));
-                            if (command.AddToCache) SetQueryCache(identity, info);
-                        }
-
-                        var func = tuple.Func;
+                        result = ReadRow<T>(info, identity, ref command, effectiveType, reader);
 
-                        object val = func(reader);
-                        if (val == null || val is T)
-                        {
-                            result = (T)val;
-                        }
-                        else
-                        {
-                            var convertToType = Nullable.GetUnderlyingType(effectiveType) ?? effectiveType;
-                            result = (T)Convert.ChangeType(val, convertToType, CultureInfo.InvariantCulture);
-                        }
                         if ((row & Row.Single) != 0 && await reader.ReadAsync(cancel).ConfigureAwait(false)) ThrowMultipleRows(row);
                         while (await reader.ReadAsync(cancel).ConfigureAwait(false)) { /* ignore rows after the first */ }
                     }

Dapper/SqlMapper.cs

@@ -12,6 +12,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Reflection.Emit;
+using System.Runtime.CompilerServices;
 using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading;
@@ -1096,14 +1097,7 @@ private static IEnumerable<T> QueryImpl<T>(this IDbConnection cnn, CommandDefini
                 while (reader.Read())
                 {
                     object val = func(reader);
-                    if (val == null || val is T)
-                    {
-                        yield return (T)val;
-                    }
-                    else
-                    {
-                        yield return (T)Convert.ChangeType(val, convertToType, CultureInfo.InvariantCulture);
-                    }
+                    yield return GetValue<T>(reader, effectiveType, val);
                 }
                 while (reader.NextResult()) { /* ignore subsequent result sets */ }
                 // happy path; close the reader cleanly - no
@@ -1179,31 +1173,14 @@ private static T QueryRowImpl<T>(IDbConnection cnn, Row row, ref CommandDefiniti
                     : CommandBehavior.SequentialAccess | CommandBehavior.SingleResult | CommandBehavior.SingleRow);
                 wasClosed = false; // *if* the connection was closed and we got this far, then we now have a reader
 
-                T result = default(T);
+                T result = default;
                 if (reader.Read() && reader.FieldCount != 0)
                 {
                     // with the CloseConnection flag, so the reader will deal with the connection; we
                     // still need something in the "finally" to ensure that broken SQL still results
                     // in the connection closing itself
-                    var tuple = info.Deserializer;
-                    int hash = GetColumnHash(reader);
-                    if (tuple.Func == null || tuple.Hash != hash)
-                    {
-                        tuple = info.Deserializer = new DeserializerState(hash, GetDeserializer(effectiveType, reader, 0, -1, false));
-                        if (command.AddToCache) SetQueryCache(identity, info);
-                    }
+                    result = ReadRow<T>(info, identity, ref command, effectiveType, reader);
 
-                    var func = tuple.Func;
-                    object val = func(reader);
-                    if (val == null || val is T)
-                    {
-                        result = (T)val;
-                    }
-                    else
-                    {
-                        var convertToType = Nullable.GetUnderlyingType(effectiveType) ?? effectiveType;
-                        result = (T)Convert.ChangeType(val, convertToType, CultureInfo.InvariantCulture);
-                    }
                     if ((row & Row.Single) != 0 && reader.Read()) ThrowMultipleRows(row);
                     while (reader.Read()) { /* ignore subsequent rows */ }
                 }
@@ -1236,6 +1213,53 @@ private static T QueryRowImpl<T>(IDbConnection cnn, Row row, ref CommandDefiniti
             }
         }
 
+        /// <summary>
+        /// Shared value deserilization path for QueryRowImpl and QueryRowAsync
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static T ReadRow<T>(CacheInfo info, Identity identity, ref CommandDefinition command, Type effectiveType, IDataReader reader)
+        {
+            var tuple = info.Deserializer;
+            int hash = GetColumnHash(reader);
+            if (tuple.Func == null || tuple.Hash != hash)
+            {
+                tuple = info.Deserializer = new DeserializerState(hash, GetDeserializer(effectiveType, reader, 0, -1, false));
+                if (command.AddToCache) SetQueryCache(identity, info);
+            }
+
+            var func = tuple.Func;
+            object val = func(reader);
+            return GetValue<T>(reader, effectiveType, val);
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static T GetValue<T>(IDataReader reader, Type effectiveType, object val)
+        {
+            if (val is T tVal)
+            {
+                return tVal;
+            }
+            else if (val == null && (!effectiveType.IsValueType || Nullable.GetUnderlyingType(effectiveType) != null))
+            {
+                return default;
+            }
+            else
+            {
+                try
+                {
+                    var convertToType = Nullable.GetUnderlyingType(effectiveType) ?? effectiveType;
+                    return (T)Convert.ChangeType(val, convertToType, CultureInfo.InvariantCulture);
+                }
+                catch (Exception ex)
+                {
+#pragma warning disable CS0618 // Type or member is obsolete
+                    ThrowDataException(ex, 0, reader, val);
+#pragma warning restore CS0618 // Type or member is obsolete
+                    return default; // For the compiler - we've already thrown
+                }
+            }
+        }
+
         /// <summary>
         /// Perform a multi-mapping query with 2 input types.
         /// This returns a single type, combined from the raw types via <paramref name="map"/>.
@@ -3619,6 +3643,11 @@ public static void ThrowDataException(Exception ex, int index, IDataReader reade
                 if (reader != null && index >= 0 && index < reader.FieldCount)
                 {
                     name = reader.GetName(index);
+                    if (name == string.Empty)
+                    {
+                        // Otherwise we throw (=value) below, which isn't intuitive
+                        name = "(Unnamed Column)";
+                    }
                     try
                     {
                         if (value == null || value is DBNull)