Log and drop signals when we can't decode the arguments (#114)

Fixes #105
temporalio · Jul 20, 2023 · 1c14789 · 1c14789
1 parent 0dc06cd
commit 1c14789
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 5 deletions.
diff --git a/src/Temporalio/Worker/WorkflowInstance.cs b/src/Temporalio/Worker/WorkflowInstance.cs
@@ -954,15 +954,30 @@ private void ApplySignalWorkflow(SignalWorkflow signal)
             // Run the handler as a top-level function
             _ = QueueNewTaskAsync(() => RunTopLevelAsync(async () =>
             {
-                await inbound.Value.HandleSignalAsync(new(
-                    Signal: signal.SignalName,
-                    Definition: signalDefn,
-                    Args: DecodeArgs(
+                // Drop the signal if we cannot decode the arguments
+                object?[] args;
+                try
+                {
+                    args = DecodeArgs(
                         method: signalDefn.Method ?? signalDefn.Delegate!.Method,
                         payloads: signal.Input,
                         itemName: $"Signal {signal.SignalName}",
                         dynamic: signalDefn.Dynamic,
-                        dynamicArgPrepend: signal.SignalName),
+                        dynamicArgPrepend: signal.SignalName);
+                }
+                catch (Exception e)
+                {
+                    logger.LogError(
+                        e,
+                        "Failed decoding signal args for {SignalName}, dropping the signal",
+                        signal.SignalName);
+                    return;
+                }
+
+                await inbound.Value.HandleSignalAsync(new(
+                    Signal: signal.SignalName,
+                    Definition: signalDefn,
+                    Args: args,
                     Headers: signal.Headers)).ConfigureAwait(true);
             }));
         }

diff --git a/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs b/tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs
@@ -887,6 +887,38 @@ await handle.SignalAsync(
         });
     }
 
+    [Workflow]
+    public class BadSignalArgsDroppedWorkflow
+    {
+        [WorkflowRun]
+        public Task RunAsync() => Workflow.DelayAsync(Timeout.Infinite);
+
+        [WorkflowSignal]
+        public async Task SomeSignalAsync(string arg) => SignalArgs.Add(arg);
+
+        [WorkflowQuery]
+        public IList<string> SignalArgs { get; } = new List<string>();
+    }
+
+    [Fact]
+    public async Task ExecuteWorkflowAsync_BadSignalArgs_ProperlyDropped()
+    {
+        await ExecuteWorkerAsync<BadSignalArgsDroppedWorkflow>(async worker =>
+        {
+            var handle = await Env.Client.StartWorkflowAsync(
+                (BadSignalArgsDroppedWorkflow wf) => wf.RunAsync(),
+                new(id: $"workflow-{Guid.NewGuid()}", taskQueue: worker.Options.TaskQueue!));
+            // Send 4 signals, the first and third being bad
+            await handle.SignalAsync("SomeSignal", new object?[] { 123 });
+            await handle.SignalAsync("SomeSignal", new object?[] { "value1" });
+            await handle.SignalAsync("SomeSignal", new object?[] { false });
+            await handle.SignalAsync("SomeSignal", new object?[] { "value2" });
+            Assert.Equal(
+                new List<string> { "value1", "value2" },
+                await handle.QueryAsync(wf => wf.SignalArgs));
+        });
+    }
+
     [Workflow]
     public class QueryWorkflow
     {