Proper handling of both zero and null tx timeout

CHANGELOG.md

@@ -0,0 +1,7 @@
+# Neo4j dotnet Driver Change Log
+
+
+## Version 5.0
+- Change Transaction config timeout to accept null, Timeout.Zero explicitly.
+    - Zero removes timout.
+    - null uses server default timeout.

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Controller.cs

@@ -106,7 +106,7 @@ public async Task Process(bool restartInitialState, Func<Exception, bool> loopCo
 					storedException = ex;
 					restartConnection = false;
 				}
-				catch (ArgumentException ex) 
+				catch (ArgumentException ex)
                 {
                     await ResponseWriter.WriteResponseAsync(ExceptionManager.GenerateExceptionResponse(ex));
 					storedException = ex;
@@ -119,7 +119,7 @@ public async Task Process(bool restartInitialState, Func<Exception, bool> loopCo
 					restartConnection = false;
                 }
 				catch (JsonSerializationException ex)
-				{	
+				{
 					await ResponseWriter.WriteResponseAsync(ExceptionManager.GenerateExceptionResponse(ex));
 					storedException = ex;
 					restartConnection = false;
@@ -130,13 +130,19 @@ public async Task Process(bool restartInitialState, Func<Exception, bool> loopCo
 					await ResponseWriter.WriteResponseAsync(ExceptionManager.GenerateExceptionResponse(ex));
 					storedException = ex;
 					restartConnection = true;
-				}					
+				}
+                catch (DriverExceptionWrapper ex)
+                {
+                    storedException = ex;
+                    await ResponseWriter.WriteResponseAsync(ExceptionManager.GenerateExceptionResponse(ex));
+                    restartConnection = false;
+                }
 				catch (IOException ex)
                 {
                     Trace.WriteLine($"Socket exception detected: {ex.Message}");    //Handled outside of the exception manager because there is no connection to reply on.
 					storedException = ex;
 					restartConnection = true;
-                }	
+                }
 				catch(Exception ex)
 				{
 					Trace.WriteLine($"General exception detected, restarting connection: {ex.Message}");
@@ -152,7 +158,7 @@ public async Task Process(bool restartInitialState, Func<Exception, bool> loopCo
                     }
                 }
                 Trace.Flush();
-            }            
+            }
         }
 
         private void BreakLoopEvent(object sender, EventArgs e)

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Exceptions/DriverExceptioinWrapper.cs

@@ -0,0 +1,7 @@
+using System;
+
+public class DriverExceptionWrapper : Exception
+{
+    public DriverExceptionWrapper() { }
+    public DriverExceptionWrapper(Exception inner) : base(null, inner) { }
+}

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Exceptions/ExceptionManager.cs

@@ -50,7 +50,7 @@ internal static class ExceptionManager
 
             { typeof(NotSupportedException),            "NotSupportedException" },
 
-			{ typeof(ArgumentException),				"ArgumentError"}	
+			{ typeof(ArgumentException),				"ArgumentError"}
         };
 
 
@@ -61,7 +61,7 @@ internal static ProtocolResponse GenerateExceptionResponse(Exception ex)
 			var type = TypeMap.GetValueOrDefault(ex.GetType());
 
 
-			//if (ex is Neo4jException || ex is NotSupportedException) 
+			//if (ex is Neo4jException || ex is NotSupportedException)
 			if(type is not null)
 			{
                 ProtocolException newError = ProtocolObjectFactory.CreateObject<ProtocolException>();
@@ -75,9 +75,19 @@ internal static ProtocolResponse GenerateExceptionResponse(Exception ex)
 					code = errorCode
 				});
             }
+            else if (ex is DriverExceptionWrapper)
+            {
+                ProtocolException newError = ProtocolObjectFactory.CreateObject<ProtocolException>();
+                return new ProtocolResponse("DriverError", new
+                {
+                    id = newError.uniqueId,
+                    errorType = ex.InnerException.GetType().Name,
+                    msg = exceptionMessage
+                });
+            }
 
             Trace.WriteLine($"Exception thrown {outerExceptionMessage}\n     which contained -- {exceptionMessage}\n{ex.StackTrace}");
             return new ProtocolResponse("BackendError", new { msg = exceptionMessage } );
         }
-    }    
+    }
 }

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionType.cs

@@ -0,0 +1,19 @@
+using System.Collections.Generic;
+using Newtonsoft.Json;
+
+namespace Neo4j.Driver.Tests.TestBackend
+{
+    internal abstract class BaseSessionType
+    {
+        public string sessionId { get; set; }
+
+        [JsonProperty(Required = Required.AllowNull)]
+        public Dictionary<string, object> txMeta { get; set; } = new Dictionary<string, object>();
+
+        [JsonProperty(Required = Required.AllowNull)]
+        public int? timeout { get; set; }
+
+        [JsonIgnore]
+        public bool TimeoutSet { get; set; }
+    }
+}

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/BaseSessionTypeJsonConverter.cs

@@ -0,0 +1,31 @@
+using System;
+using System.Collections.Generic;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+
+namespace Neo4j.Driver.Tests.TestBackend
+{
+    internal class BaseSessionTypeJsonConverter<T> : JsonConverter<T> where T: BaseSessionType, new()
+    {
+        public override void WriteJson(JsonWriter writer, T value, JsonSerializer serializer) =>
+            throw new NotImplementedException();
+
+        public override T ReadJson(JsonReader reader, Type objectType,
+            T existingValue, bool hasExistingValue, JsonSerializer serializer)
+        {
+            var json = JObject.Load(reader);
+            var sessionSettings = new T();
+            SetBaseValues(json, sessionSettings);
+            return sessionSettings;
+        }
+
+        protected void SetBaseValues(JObject jsonObject, T baseSession)
+        {
+            baseSession.sessionId = jsonObject["sessionId"]?.Value<string>();
+            baseSession.txMeta = jsonObject["txMeta"]?.ToObject<Dictionary<string, object>>()
+                                 ?? new Dictionary<string, object>();
+            baseSession.timeout = jsonObject["timeout"]?.Value<int?>();
+            baseSession.TimeoutSet = jsonObject.ContainsKey("timeout");
+        }
+    }
+}

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/SessionBeginTransaction.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Threading.Tasks;
-using System.Collections.Generic;
 using Newtonsoft.Json;
 
 namespace Neo4j.Driver.Tests.TestBackend
@@ -11,23 +10,27 @@ internal class SessionBeginTransaction : IProtocolObject
         [JsonIgnore]
         public string TransactionId { get; set; }
 
-		public class SessionBeginTransactionType
-		{
-			public string sessionId { get; set; }
-
-			[JsonProperty(Required = Required.AllowNull)]
-			public Dictionary<string, object> txMeta { get; set; } = new Dictionary<string, object>();
 
-			[JsonProperty(Required = Required.AllowNull)]
-			public int timeout { get; set; } = -1;
+        [JsonConverter(typeof(BaseSessionTypeJsonConverter<SessionBeginTransactionType>))]
+        public class SessionBeginTransactionType : BaseSessionType
+        {
         }
 
         void TransactionConfig(TransactionConfigBuilder configBuilder)
         {
-            if (data.timeout != -1)
+            try
+            {
+                if (data.TimeoutSet)
+                {
+                    var timeout = data.timeout.HasValue
+                    ? TimeSpan.FromMilliseconds(data.timeout.Value)
+                        : default(TimeSpan?);
+                    configBuilder.WithTimeout(timeout);
+                }
+            }
+            catch (ArgumentOutOfRangeException e) when ((data.timeout ?? 0) < 0 && e.ParamName == "value")
             {
-                var time = TimeSpan.FromMilliseconds(data.timeout);
-                configBuilder.WithTimeout(time);
+                throw new DriverExceptionWrapper(e);
             }
 
             if (data.txMeta.Count > 0) configBuilder.WithMetadata(data.txMeta);

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/SessionReadTransaction.cs

@@ -12,17 +12,10 @@ internal class SessionReadTransaction : IProtocolObject
         private string TransactionId { get; set; }
 
 
-        public class SessionReadTransactionType
+        [JsonConverter(typeof(BaseSessionTypeJsonConverter<SessionReadTransactionType>))]
+        public class SessionReadTransactionType : BaseSessionType
         {
-            public string sessionId { get; set; }
-
-            public string cypher { get; set; }
-
-			public int timeout { get; set; } = 0;
-
-			[JsonProperty(Required = Required.AllowNull)]
-			public Dictionary<string, object> txMeta { get; set; } = new Dictionary<string, object>();
-		}
+        }
 
         public override async Task Process(Controller controller)
         {
@@ -55,9 +48,9 @@ await controller.Process(false, e =>
 							return false;
 						case NewSession.SessionState.RetryAbleNegative:
 							throw e;
-							
+
 						default:
-							return true;						
+							return true;
 					}
 				});
 
@@ -89,7 +82,20 @@ void TransactionConfig(TransactionConfigBuilder configBuilder)
 		{
 			if (data.txMeta.Count > 0) configBuilder.WithMetadata(data.txMeta);
 
-			if (data.timeout > 0) configBuilder.WithTimeout(TimeSpan.FromSeconds(data.timeout));
+            try
+            {
+                if (data.TimeoutSet)
+                {
+                    var timeout = data.timeout.HasValue
+                    ? TimeSpan.FromMilliseconds(data.timeout.Value)
+                        : default(TimeSpan?);
+                    configBuilder.WithTimeout(timeout);
+                }
+            }
+            catch (ArgumentOutOfRangeException e) when ((data.timeout ?? 0) < 0 && e.ParamName == "value")
+            {
+                throw new DriverExceptionWrapper(e);
+            }
 		}
 	}
 }

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/SessionRun.cs

@@ -1,59 +1,61 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
-using Newtonsoft.Json.Serialization;
 using Newtonsoft.Json;
-using Neo4j.Driver;
 
 namespace Neo4j.Driver.Tests.TestBackend
 {
-    internal class SessionRun : IProtocolObject
+	internal class SessionRun : IProtocolObject
     {
         public SessionRunType data { get; set; } = new SessionRunType();
+
         [JsonIgnore]
         private string ResultId { get; set; }
 
-        public class SessionRunType
+        [JsonConverter(typeof(SessionTypeJsonConverter))]
+        public class SessionRunType : BaseSessionType
         {
-            public string sessionId { get; set; }
-
             public string cypher { get; set; }
 
             [JsonProperty("params")]
             public Dictionary<string, CypherToNativeObject> parameters { get; set; } = new Dictionary<string, CypherToNativeObject>();
-
-            [JsonProperty(Required = Required.AllowNull)]
-            public Dictionary<string, object> txMeta { get; set; } = new Dictionary<string, object>();
-
-            [JsonProperty(Required = Required.AllowNull)]
-            public int timeout { get; set; } = -1;
-
         }
 
         private Dictionary<string, object> ConvertParameters(Dictionary<string, CypherToNativeObject> source)
-		{
+        {
             if (data.parameters == null)
                 return null;
 
             Dictionary<string, object> newParams = new Dictionary<string, object>();
 
             foreach(KeyValuePair<string, CypherToNativeObject> element in source)
-			{
+            {
                 newParams.Add(element.Key, CypherToNative.Convert(element.Value));
-			}
+            }
 
             return newParams;
-		}
+        }
 
         void TransactionConfig(TransactionConfigBuilder configBuilder)
         {
-            if (data.timeout != -1)
+            try
+            {
+                if (data.TimeoutSet)
+                {
+                    var timeout = data.timeout.HasValue
+                    ? TimeSpan.FromMilliseconds(data.timeout.Value)
+                        : default(TimeSpan?);
+                    configBuilder.WithTimeout(timeout);
+                }
+            }
+            catch (ArgumentOutOfRangeException e) when ((data.timeout ?? 0) < 0 && e.ParamName == "value")
             {
-                var time = TimeSpan.FromMilliseconds(data.timeout);
-                configBuilder.WithTimeout(time);
+                throw new DriverExceptionWrapper(e);
             }
 
-            if (data.txMeta.Count > 0) configBuilder.WithMetadata(data.txMeta);
+            if (data.txMeta.Count > 0) 
+                configBuilder.WithMetadata(data.txMeta);
         }
 
         public override async Task Process()
@@ -62,9 +64,9 @@ public override async Task Process()
             IResultCursor cursor = await newSession.Session.RunAsync(data.cypher, ConvertParameters(data.parameters), TransactionConfig).ConfigureAwait(false);
 
             var result = ProtocolObjectFactory.CreateObject<Result>();
-			result.ResultCursor = cursor;
+            result.ResultCursor = cursor;
 
-			ResultId = result.uniqueId;
+            ResultId = result.uniqueId;
         }
 
         public override string Respond()

Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/Protocol/Session/SessionTypeJsonConverter.cs

@@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+
+namespace Neo4j.Driver.Tests.TestBackend
+{
+    internal class SessionTypeJsonConverter : BaseSessionTypeJsonConverter<SessionRun.SessionRunType>
+	{
+		public override void WriteJson(JsonWriter writer, SessionRun.SessionRunType value, JsonSerializer serializer) =>
+			throw new NotImplementedException();
+
+		public override SessionRun.SessionRunType ReadJson(JsonReader reader, Type objectType,
+			SessionRun.SessionRunType existingValue, bool hasExistingValue, JsonSerializer serializer)
+		{
+			var json = JObject.Load(reader);
+			var sessionType = new SessionRun.SessionRunType
+			{
+				cypher = json["cypher"]?.Value<string>(),
+				parameters = json["params"]?.ToObject<Dictionary<string, CypherToNativeObject>>() 
+								?? new Dictionary<string, CypherToNativeObject>(),
+			};
+            SetBaseValues(json, sessionType);
+            return sessionType;
+        }
+	}
+}