Fix some issues with Ignitor (#15276)

pranavkm · web-flow · commit 9f9fbd0290f5 · 2019-10-23T15:40:27.000-07:00
* Avoid null ref when FormatException is not set
* Handle errors when the client is canceled but an operation is in progress
diff --git a/src/Components/Ignitor/src/BlazorClient.cs b/src/Components/Ignitor/src/BlazorClient.cs
diff --git a/src/Components/Ignitor/src/CancellableOperation.cs b/src/Components/Ignitor/src/CancellableOperation.cs
@@ -5,37 +5,50 @@
 using System.Threading;
 using System.Threading.Tasks;
 
+#nullable enable
 namespace Ignitor
 {
     internal class CancellableOperation<TResult>
     {
-        public CancellableOperation(TimeSpan? timeout)
+        public CancellableOperation(TimeSpan? timeout, CancellationToken cancellationToken)
         {
             Timeout = timeout;
 
             Completion = new TaskCompletionSource<TResult>(TaskContinuationOptions.RunContinuationsAsynchronously);
             Completion.Task.ContinueWith(
                 (task, state) =>
                 {
-                    var operation = (CancellableOperation<TResult>)state;
+                    var operation = (CancellableOperation<TResult>)state!;
                     operation.Dispose();
                 },
                 this,
                 TaskContinuationOptions.ExecuteSynchronously); // We need to execute synchronously to clean-up before anything else continues
 
+
+            Cancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+
             if (Timeout != null && Timeout != System.Threading.Timeout.InfiniteTimeSpan && Timeout != TimeSpan.MaxValue)
             {
-                Cancellation = new CancellationTokenSource(Timeout.Value);
-                CancellationRegistration = Cancellation.Token.Register(
-                    (self) =>
-                    {
-                        var operation = (CancellableOperation<TResult>)self;
-                        operation.Completion.TrySetCanceled(operation.Cancellation.Token);
-                        operation.Cancellation.Dispose();
-                        operation.CancellationRegistration.Dispose();
-                    },
-                    this);
+                Cancellation.CancelAfter(Timeout.Value);
             }
+
+            CancellationRegistration = Cancellation.Token.Register(
+                (self) =>
+                {
+                    var operation = (CancellableOperation<TResult>)self!;
+
+                    if (cancellationToken.IsCancellationRequested)
+                    {
+                        // The operation was externally canceled before it timed out.
+                        Dispose();
+                        return;
+                    }
+
+                    operation.Completion.TrySetException(new TimeoutException($"The operation timed out after {Timeout}."));
+                    operation.Cancellation?.Dispose();
+                    operation.CancellationRegistration.Dispose();
+                },
+                this);
         }
 
         public TimeSpan? Timeout { get; }
@@ -62,3 +75,4 @@ private void Dispose()
         }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/ComponentState.cs b/src/Components/Ignitor/src/ComponentState.cs
@@ -3,6 +3,7 @@
 
 namespace Ignitor
 {
+#nullable enable
     public class ComponentState
     {
         public ComponentState(int componentId)
@@ -11,6 +12,7 @@ public ComponentState(int componentId)
         }
 
         public int ComponentId { get; }
-        public IComponent Component { get; }
+        public IComponent? Component { get; }
     }
+#nullable restore
 }
diff --git a/src/Components/Ignitor/src/ContainerNode.cs b/src/Components/Ignitor/src/ContainerNode.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 
+#nullable enable
 namespace Ignitor
 {
     public abstract class ContainerNode : Node
@@ -82,3 +83,4 @@ public void RemoveLogicalChild(int childIndex)
         }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/ElementHive.cs b/src/Components/Ignitor/src/ElementHive.cs
@@ -3,7 +3,9 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 
+#nullable enable
 namespace Ignitor
 {
     public class ElementHive
@@ -35,7 +37,7 @@ public void Update(RenderBatch batch)
             }
         }
 
-        public bool TryFindElementById(string id, out ElementNode element)
+        public bool TryFindElementById(string id, [NotNullWhen(true)] out ElementNode? element)
         {
             foreach (var kvp in Components)
             {
@@ -49,7 +51,7 @@ public bool TryFindElementById(string id, out ElementNode element)
             element = null;
             return false;
 
-            bool TryGetElementFromChildren(Node node, out ElementNode foundNode)
+            bool TryGetElementFromChildren(Node node, out ElementNode? foundNode)
             {
                 if (node is ElementNode elementNode &&
                     elementNode.Attributes.TryGetValue("id", out var elementId) &&
@@ -81,6 +83,7 @@ private void UpdateComponent(RenderBatch batch, int componentId, ArrayBuilderSeg
             {
                 component = new ComponentNode(componentId);
                 Components.Add(componentId, component);
+
             }
 
             ApplyEdits(batch, component, 0, edits);
@@ -199,7 +202,7 @@ private void ApplyEdits(RenderBatch batch, ContainerNode parent, int childIndex,
 
                     case RenderTreeEditType.StepOut:
                         {
-                            parent = parent.Parent;
+                            parent = parent.Parent ?? throw new InvalidOperationException($"Cannot step out of {parent}");
                             currentDepth--;
                             childIndexAtCurrentDepth = currentDepth == 0 ? childIndex : 0; // The childIndex is only ever nonzero at zero depth
                             break;
@@ -469,3 +472,4 @@ public PermutationListEntry(int from, int to)
         }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/ElementNode.cs b/src/Components/Ignitor/src/ElementNode.cs
@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.SignalR.Client;
 
+#nullable enable
 namespace Ignitor
 {
     public class ElementNode : ContainerNode
@@ -87,7 +88,7 @@ internal Task SelectAsync(HubConnection connection, string value)
             return DispatchEventCore(connection, Serialize(webEventDescriptor), Serialize(args));
         }
 
-        public Task ClickAsync(HubConnection connection)
+        internal Task ClickAsync(HubConnection connection)
         {
             if (!Events.TryGetValue("click", out var clickEventDescriptor))
             {
@@ -129,3 +130,4 @@ public ElementEventDescriptor(string eventName, ulong eventId)
         }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/Error.cs b/src/Components/Ignitor/src/Error.cs
@@ -1,10 +1,12 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+#nullable enable
 namespace Ignitor
 {
     public class Error
     {
-        public string Stack { get; set; }
+        public string? Stack { get; set; }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/IComponent.cs b/src/Components/Ignitor/src/IComponent.cs
@@ -1,6 +1,5 @@
-using System;
-using System.Collections.Generic;
-using System.Text;
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 namespace Ignitor
 {
diff --git a/src/Components/Ignitor/src/Node.cs b/src/Components/Ignitor/src/Node.cs
@@ -1,10 +1,14 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+#nullable enable
+
 namespace Ignitor
 {
     public abstract class Node
     {
-        public virtual ContainerNode Parent { get; set; }
+        public virtual ContainerNode? Parent { get; set; }
     }
 }
+
+#nullable restore
diff --git a/src/Components/Ignitor/src/NodeSerializer.cs b/src/Components/Ignitor/src/NodeSerializer.cs
@@ -4,6 +4,8 @@
 using System;
 using System.IO;
 
+#nullable enable
+
 namespace Ignitor
 {
     internal static class NodeSerializer
@@ -96,7 +98,7 @@ private void SerializeElement(ElementNode elementNode)
                     if (attribute.Value != null)
                     {
                         Write("=\"");
-                        Write(attribute.Value.ToString());
+                        Write(attribute.Value.ToString()!);
                         Write("\"");
                     }
                 }
@@ -113,7 +115,7 @@ private void SerializeElement(ElementNode elementNode)
                         if (properties.Value != null)
                         {
                             Write("=\"");
-                            Write(properties.Value.ToString());
+                            Write(properties.Value.ToString()!);
                             Write("\"");
                         }
                     }
@@ -194,3 +196,4 @@ private void WriteIndent()
         }
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/Operations.cs b/src/Components/Ignitor/src/Operations.cs
@@ -3,6 +3,7 @@
 
 using System.Collections.Concurrent;
 
+#nullable enable
 namespace Ignitor
 {
     public sealed class Operations
@@ -18,3 +19,4 @@ public sealed class Operations
         public ConcurrentQueue<CapturedJSInteropCall> JSInteropCalls { get; } = new ConcurrentQueue<CapturedJSInteropCall>();
     }
 }
+#nullable restore
diff --git a/src/Components/Ignitor/src/RenderBatchReader.cs b/src/Components/Ignitor/src/RenderBatchReader.cs
@@ -4,6 +4,8 @@
 using System;
 using System.Text;
 
+#nullable enable
+
 namespace Ignitor
 {
     public static class RenderBatchReader
@@ -206,7 +208,7 @@ private static ArrayRange<ulong> ReadDisposedEventHandlerIds(ReadOnlySpan<byte>
             return new ArrayRange<ulong>(Array.Empty<ulong>(), 0);
         }
 
-        private static string ReadString(ReadOnlySpan<byte> data, string[] strings)
+        private static string? ReadString(ReadOnlySpan<byte> data, string[] strings)
         {
             var index = BitConverter.ToInt32(data.Slice(0, 4));
             return index >= 0 ? strings[index] : null;
@@ -279,3 +281,4 @@ public ReadOnlySpan<byte> GetStringTableIndexes(ReadOnlySpan<byte> data)
         }
     }
 }
+#nullable restore

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`namespace Ignitor`
`5`	`5`	`{`
	`6`	`+#nullable enable`
`6`	`7`	`public class ComponentState`
`7`	`8`	`{`
`8`	`9`	`public ComponentState(int componentId)`
`@@ -11,6 +12,7 @@ public ComponentState(int componentId)`
`11`	`12`	`}`
`12`	`13`
`13`	`14`	`public int ComponentId { get; }`
`14`		`- public IComponent Component { get; }`
	`15`	`+ public IComponent? Component { get; }`
`15`	`16`	`}`
	`17`	`+#nullable restore`
`16`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`using System;`
`5`	`5`	`using System.Collections.Generic;`
`6`	`6`
	`7`	`+#nullable enable`
`7`	`8`	`namespace Ignitor`
`8`	`9`	`{`
`9`	`10`	`public abstract class ContainerNode : Node`
`@@ -82,3 +83,4 @@ public void RemoveLogicalChild(int childIndex)`
`82`	`83`	`}`
`83`	`84`	`}`
`84`	`85`	`}`
	`86`	`+#nullable restore`
Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,9 @@`
`3`	`3`
`4`	`4`	`using System;`
`5`	`5`	`using System.Collections.Generic;`
	`6`	`+using System.Diagnostics.CodeAnalysis;`
`6`	`7`
	`8`	`+#nullable enable`
`7`	`9`	`namespace Ignitor`
`8`	`10`	`{`
`9`	`11`	`public class ElementHive`
`@@ -35,7 +37,7 @@ public void Update(RenderBatch batch)`
`35`	`37`	`}`
`36`	`38`	`}`
`37`	`39`
`38`		`- public bool TryFindElementById(string id, out ElementNode element)`
	`40`	`+ public bool TryFindElementById(string id, [NotNullWhen(true)] out ElementNode? element)`
`39`	`41`	`{`
`40`	`42`	`foreach (var kvp in Components)`
`41`	`43`	`{`
`@@ -49,7 +51,7 @@ public bool TryFindElementById(string id, out ElementNode element)`
`49`	`51`	`element = null;`
`50`	`52`	`return false;`
`51`	`53`
`52`		`- bool TryGetElementFromChildren(Node node, out ElementNode foundNode)`
	`54`	`+ bool TryGetElementFromChildren(Node node, out ElementNode? foundNode)`
`53`	`55`	`{`
`54`	`56`	`if (node is ElementNode elementNode &&`
`55`	`57`	`elementNode.Attributes.TryGetValue("id", out var elementId) &&`
`@@ -81,6 +83,7 @@ private void UpdateComponent(RenderBatch batch, int componentId, ArrayBuilderSeg`
`81`	`83`	`{`
`82`	`84`	`component = new ComponentNode(componentId);`
`83`	`85`	`Components.Add(componentId, component);`
	`86`	`+`
`84`	`87`	`}`
`85`	`88`
`86`	`89`	`ApplyEdits(batch, component, 0, edits);`
`@@ -199,7 +202,7 @@ private void ApplyEdits(RenderBatch batch, ContainerNode parent, int childIndex,`
`199`	`202`
`200`	`203`	`case RenderTreeEditType.StepOut:`
`201`	`204`	`{`
`202`		`- parent = parent.Parent;`
	`205`	`+ parent = parent.Parent ?? throw new InvalidOperationException($"Cannot step out of {parent}");`
`203`	`206`	`currentDepth--;`
`204`	`207`	`childIndexAtCurrentDepth = currentDepth == 0 ? childIndex : 0; // The childIndex is only ever nonzero at zero depth`
`205`	`208`	`break;`
`@@ -469,3 +472,4 @@ public PermutationListEntry(int from, int to)`
`469`	`472`	`}`
`470`	`473`	`}`
`471`	`474`	`}`
	`475`	`+#nullable restore`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@`
`7`	`7`	`using System.Threading.Tasks;`
`8`	`8`	`using Microsoft.AspNetCore.SignalR.Client;`
`9`	`9`
	`10`	`+#nullable enable`
`10`	`11`	`namespace Ignitor`
`11`	`12`	`{`
`12`	`13`	`public class ElementNode : ContainerNode`
`@@ -87,7 +88,7 @@ internal Task SelectAsync(HubConnection connection, string value)`
`87`	`88`	`return DispatchEventCore(connection, Serialize(webEventDescriptor), Serialize(args));`
`88`	`89`	`}`
`89`	`90`
`90`		`- public Task ClickAsync(HubConnection connection)`
	`91`	`+ internal Task ClickAsync(HubConnection connection)`
`91`	`92`	`{`
`92`	`93`	`if (!Events.TryGetValue("click", out var clickEventDescriptor))`
`93`	`94`	`{`
`@@ -129,3 +130,4 @@ public ElementEventDescriptor(string eventName, ulong eventId)`
`129`	`130`	`}`
`130`	`131`	`}`
`131`	`132`	`}`
	`133`	`+#nullable restore`
Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,12 @@`
`1`	`1`	`// Copyright (c) .NET Foundation. All rights reserved.`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
	`4`	`+#nullable enable`
`4`	`5`	`namespace Ignitor`
`5`	`6`	`{`
`6`	`7`	`public class Error`
`7`	`8`	`{`
`8`		`- public string Stack { get; set; }`
	`9`	`+ public string? Stack { get; set; }`
`9`	`10`	`}`
`10`	`11`	`}`
	`12`	`+#nullable restore`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,5 @@`
`1`		`-using System;`
`2`		`-using System.Collections.Generic;`
`3`		`-using System.Text;`
	`1`	`+// Copyright (c) .NET Foundation. All rights reserved.`
	`2`	`+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`4`	`3`
`5`	`4`	`namespace Ignitor`
`6`	`5`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,14 @@`
`1`	`1`	`// Copyright (c) .NET Foundation. All rights reserved.`
`2`	`2`	`// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.`
`3`	`3`
	`4`	`+#nullable enable`
	`5`	`+`
`4`	`6`	`namespace Ignitor`
`5`	`7`	`{`
`6`	`8`	`public abstract class Node`
`7`	`9`	`{`
`8`		`- public virtual ContainerNode Parent { get; set; }`
	`10`	`+ public virtual ContainerNode? Parent { get; set; }`
`9`	`11`	`}`
`10`	`12`	`}`
	`13`	`+`
	`14`	`+#nullable restore`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,8 @@`
`4`	`4`	`using System;`
`5`	`5`	`using System.IO;`
`6`	`6`
	`7`	`+#nullable enable`
	`8`	`+`
`7`	`9`	`namespace Ignitor`
`8`	`10`	`{`
`9`	`11`	`internal static class NodeSerializer`
`@@ -96,7 +98,7 @@ private void SerializeElement(ElementNode elementNode)`
`96`	`98`	`if (attribute.Value != null)`
`97`	`99`	`{`
`98`	`100`	`Write("=\"");`
`99`		`- Write(attribute.Value.ToString());`
	`101`	`+ Write(attribute.Value.ToString()!);`
`100`	`102`	`Write("\"");`
`101`	`103`	`}`
`102`	`104`	`}`
`@@ -113,7 +115,7 @@ private void SerializeElement(ElementNode elementNode)`
`113`	`115`	`if (properties.Value != null)`
`114`	`116`	`{`
`115`	`117`	`Write("=\"");`
`116`		`- Write(properties.Value.ToString());`
	`118`	`+ Write(properties.Value.ToString()!);`
`117`	`119`	`Write("\"");`
`118`	`120`	`}`
`119`	`121`	`}`
`@@ -194,3 +196,4 @@ private void WriteIndent()`
`194`	`196`	`}`
`195`	`197`	`}`
`196`	`198`	`}`
	`199`	`+#nullable restore`