Fix for #3084, the default internal serializer tried to mimic json.net deserialize() behaviour for empty strings with a bazooka try catch around JToken.LoadAsync which ended up hiding real deserialization errors, **sigh** (#3086)

Author: Mpdreamz

src/Elasticsearch.Net/Transport/Pipeline/ResponseBuilder.cs

@@ -10,7 +10,7 @@ namespace Elasticsearch.Net
 {
 	public static class ResponseBuilder
 	{
-		private const int BufferSize = 81920;
+		public const int BufferSize = 81920;
 
 		public static TResponse ToResponse<TResponse>(
 			RequestData requestData,

src/Nest/CommonAbstractions/SerializationBehavior/InternalSerializer.cs

@@ -98,41 +98,25 @@ public object Deserialize(Type type, Stream stream)
 		public async Task<T> DeserializeAsync<T>(Stream stream, CancellationToken cancellationToken = default(CancellationToken))
 		{
 			if (stream == null) return default(T);
-			using (var streamReader = new StreamReader(stream))
-			using (var jsonTextReader = new JsonTextReader(streamReader))
-			{
-				//JsonSerializerInternalReader that's is used in `Deserialize()` from the synchronous codepath
-				// has the same try catch
-				// https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs#L145
-				// :/
-				try
-				{
-					var token = await JToken.LoadAsync(jsonTextReader, cancellationToken).ConfigureAwait(false);
-					return token.ToObject<T>(this.Serializer);
-				}
-				catch
-				{
-					return default(T);
-				}
-			}
+			var bytes = await ReadToBytesAsync(stream, cancellationToken);
+
+			if (bytes == null || bytes.Length == 0) return default(T);
+            using (var ms  = new MemoryStream(bytes))
+            using (var sr = new StreamReader(ms))
+            using (var jtr = new JsonTextReader(sr))
+				return this.Serializer.Deserialize<T>(jtr);
 		}
 
 		public async Task<object> DeserializeAsync(Type type, Stream stream, CancellationToken cancellationToken = default(CancellationToken))
 		{
 			if (stream == null) return type.DefaultValue();
-			using (var streamReader = new StreamReader(stream))
-			using (var jsonTextReader = new JsonTextReader(streamReader))
-			{
-				try
-				{
-					var token = await JToken.LoadAsync(jsonTextReader, cancellationToken).ConfigureAwait(false);
-					return token.ToObject(type, this.Serializer);
-				}
-				catch
-				{
-					return type.DefaultValue();
-				}
-			}
+			var bytes = await ReadToBytesAsync(stream, cancellationToken);
+
+			if (bytes == null || bytes.Length == 0) return type.DefaultValue();
+            using (var ms  = new MemoryStream(bytes))
+            using (var sr = new StreamReader(ms))
+            using (var jtr = new JsonTextReader(sr))
+				return this.Serializer.Deserialize(jtr, type);
 		}
 
 		private JsonSerializerSettings CreateSettings(SerializationFormatting formatting)
@@ -150,5 +134,15 @@ private JsonSerializerSettings CreateSettings(SerializationFormatting formatting
 
 			return settings;
 		}
+
+		private static async Task<byte[]> ReadToBytesAsync(Stream stream, CancellationToken cancellationToken)
+		{
+			if (stream is MemoryStream o) return o.ToArray();
+			using (var ms = new MemoryStream())
+			{
+				await stream.CopyToAsync(ms, ResponseBuilder.BufferSize, cancellationToken);
+				return ms.ToArray();
+			}
+		}
 	}
 }

src/Tests/Reproduce/GithubIssue3084.cs

@@ -0,0 +1,51 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Elasticsearch.Net;
+using FluentAssertions;
+using Nest;
+using Tests.Framework;
+using Tests.Framework.ManagedElasticsearch.Clusters;
+using Tests.Framework.MockData;
+using Xunit;
+
+namespace Tests.Reproduce
+{
+	public class GithubIssue3084 : IClusterFixture<WritableCluster>
+	{
+		private readonly WritableCluster _cluster;
+
+		public GithubIssue3084(WritableCluster cluster) => _cluster = cluster;
+
+		protected static string RandomString() => Guid.NewGuid().ToString("N").Substring(0, 8);
+
+		public class ObjectVersion1
+		{
+			public int Id { get; set; }
+			public double Numeric { get; set; }
+		}
+		public class ObjectVersion2
+		{
+			public int Id { get; set; }
+			public int Numeric { get; set; }
+		}
+
+
+		[I]
+		public void DeserializeErrorIsTheSameForAsync()
+		{
+			var client = _cluster.Client;
+			var index = $"gh3084-{RandomString()}";
+			var document = new ObjectVersion1 {Id = 1, Numeric = 0};
+
+			var indexResult = client.Index(document, i => i.Index(index).Type("doc"));
+			indexResult.ShouldBeValid();
+
+			Action getDoc = () =>client.Get<ObjectVersion2>(new GetRequest(index, "doc", 1));
+			Func<Task<IGetResponse<ObjectVersion2>>> getDocAsync = async () => await client.GetAsync<ObjectVersion2>(new GetRequest(index, "doc", 1));
+
+			getDoc.ShouldThrow<Exception>("synchonous code path should throw");
+			getDocAsync.ShouldThrow<Exception>("async code path should throw");
+		}
+	}
+}