Merge pull request #20 from DeNA/stop-when-error-immediately-thrown

Fix not stopping and reporting when error occurred
DeNA · Nov 14, 2023 · 7d98f68 · 7d98f68
2 parents 145f567 + 9af66be
commit 7d98f68
Show file tree

Hide file tree

Showing 18 changed files with 742 additions and 15 deletions.
diff --git a/Editor/DeNA.Anjin.Editor.asmdef b/Editor/DeNA.Anjin.Editor.asmdef
@@ -2,6 +2,7 @@
     "name": "DeNA.Anjin.Editor",
     "rootNamespace": "DeNA.Anjin",
     "references": [
+        "UniTask",
         "DeNA.Anjin",
         "TestHelper.Monkey.Annotations",
         "TestHelper.Monkey"

diff --git a/Editor/UI/Settings/AutopilotSettingsEditor.cs b/Editor/UI/Settings/AutopilotSettingsEditor.cs
@@ -2,6 +2,7 @@
 // This software is released under the MIT License.
 
 using System.Diagnostics.CodeAnalysis;
+using Cysharp.Threading.Tasks;
 using DeNA.Anjin.Settings;
 using UnityEditor;
 using UnityEngine;
@@ -137,7 +138,7 @@ public override void OnInspectorGUI()
             {
                 if (GUILayout.Button(s_stopButton))
                 {
-                    Stop();
+                    Stop().Forget();
                 }
             }
             else
@@ -157,10 +158,10 @@ private static void DrawHeader(string label)
         }
 
         [SuppressMessage("ApiDesign", "RS0030")]
-        internal void Stop()
+        internal async UniTask Stop()
         {
             var autopilot = FindObjectOfType<Autopilot>();
-            autopilot.Terminate(Autopilot.ExitCode.Normally);
+            await autopilot.TerminateAsync(Autopilot.ExitCode.Normally);
         }
 
         internal void Launch(AutopilotState state)

diff --git a/Runtime/AgentDispatcher.cs b/Runtime/AgentDispatcher.cs
@@ -1,6 +1,7 @@
 // Copyright (c) 2023 DeNA Co., Ltd.
 // This software is released under the MIT License.
 
+using System;
 using DeNA.Anjin.Agents;
 using DeNA.Anjin.Settings;
 using DeNA.Anjin.Utilities;
@@ -106,9 +107,17 @@ private void DispatchAgent(AbstractAgent agent)
 
             agent.Logger = _logger;
             agent.Random = _randomFactory.CreateRandom();
-            agent.Run(token).Forget();
-
-            _logger.Log($"Agent {agentName} dispatched!");
+
+            try
+            {
+                agent.Run(token).Forget();
+                _logger.Log($"Agent {agentName} dispatched!");
+            }
+            catch (Exception e)
+            {
+                Debug.LogException(e);
+                _logger.Log($"Agent {agentName} dispatched but immediately threw an error!");
+            }
         }
     }
 }
diff --git a/Runtime/Autopilot.cs b/Runtime/Autopilot.cs
@@ -3,6 +3,8 @@
 
 using System;
 using System.Collections;
+using System.Threading;
+using Cysharp.Threading.Tasks;
 using DeNA.Anjin.Reporters;
 using DeNA.Anjin.Settings;
 using DeNA.Anjin.Utilities;
@@ -62,15 +64,18 @@ private void Start()
 
             _randomFactory = new RandomFactory(seed);
             _logger.Log($"Random seed is {seed}");
+
+            // NOTE: Registering logMessageReceived must be placed before DispatchByScene.
+            //       Because some agent can throw an error immediately, so reporter can miss the error if
+            //       registering logMessageReceived is placed after DispatchByScene.
+            _reporter = new SlackReporter(_settings, new SlackAPI());
+            _logMessageHandler = new LogMessageHandler(_settings, _reporter);
+            Application.logMessageReceived += _logMessageHandler.HandleLog;
 
             _dispatcher = new AgentDispatcher(_settings, _logger, _randomFactory);
             _dispatcher.DispatchByScene(SceneManager.GetActiveScene());
             SceneManager.activeSceneChanged += _dispatcher.DispatchByScene;
 
-            _reporter = new SlackReporter(_settings, new SlackAPI());
-            _logMessageHandler = new LogMessageHandler(_settings, _reporter);
-            Application.logMessageReceived += _logMessageHandler.HandleLog;
-
             if (_settings.lifespanSec > 0)
             {
                 StartCoroutine(Lifespan(_settings.lifespanSec));
@@ -92,7 +97,7 @@ private void Start()
         private IEnumerator Lifespan(int timeoutSec)
         {
             yield return new WaitForSecondsRealtime(timeoutSec);
-            Terminate(ExitCode.Normally);
+            yield return UniTask.ToCoroutine(() => TerminateAsync(ExitCode.Normally));
         }
 
         /// <summary>
@@ -101,7 +106,8 @@ private IEnumerator Lifespan(int timeoutSec)
         /// <param name="exitCode">Exit code for Unity Editor</param>
         /// <param name="logString">Log message string when terminate by the log message</param>
         /// <param name="stackTrace">Stack trace when terminate by the log message</param>
-        public void Terminate(ExitCode exitCode, string logString = null, string stackTrace = null)
+        /// <returns>A task awaits termination get completed</returns>
+        public async UniTask TerminateAsync(ExitCode exitCode, string logString = null, string stackTrace = null, CancellationToken token = default)
         {
             if (_dispatcher != null)
             {
@@ -132,12 +138,29 @@ public void Terminate(ExitCode exitCode, string logString = null, string stackTr
             // Terminate when ran specified time.
             _logger.Log("Stop playing by autopilot");
             _state.exitCode = exitCode;
+            // XXX: Avoid a problem that Editor stay playing despite isPlaying get assigned false.
+            // SEE: https://github.com/DeNA/Anjin/issues/20
+            await UniTask.DelayFrame(1, cancellationToken: token);
             UnityEditor.EditorApplication.isPlaying = false;
             // Call Launcher.OnChangePlayModeState() so terminates Unity editor, when launch from CLI.
 #else
             _logger.Log($"Exit Unity-editor by autopilot, exit code={exitCode}");
             Application.Quit((int)exitCode);
+            await UniTask.CompletedTask;
 #endif
         }
+
+
+        /// <summary>
+        /// Terminate autopilot
+        /// </summary>
+        /// <param name="exitCode">Exit code for Unity Editor</param>
+        /// <param name="logString">Log message string when terminate by the log message</param>
+        /// <param name="stackTrace">Stack trace when terminate by the log message</param>
+        [Obsolete("Use " + nameof(TerminateAsync))]
+        public void Terminate(ExitCode exitCode, string logString = null, string stackTrace = null)
+        {
+            TerminateAsync(exitCode, logString, stackTrace).Forget();
+        }
     }
 }
diff --git a/Runtime/Utilities/LogMessageHandler.cs b/Runtime/Utilities/LogMessageHandler.cs
@@ -44,7 +44,7 @@ public async void HandleLog(string logString, string stackTrace, LogType type)
             var autopilot = Object.FindObjectOfType<Autopilot>();
             if (autopilot != null)
             {
-                autopilot.Terminate(Autopilot.ExitCode.AutopilotFailed, logString, stackTrace);
+                await autopilot.TerminateAsync(Autopilot.ExitCode.AutopilotFailed, logString, stackTrace);
             }
         }
 

diff --git a/Tests/Runtime/LauncherTest.cs b/Tests/Runtime/LauncherTest.cs
@@ -39,7 +39,7 @@ public async Task Launch_InPlayMode_RunAutopilot()
             var autopilot = Object.FindObjectOfType<Autopilot>();
             Assert.That((bool)autopilot, Is.True, "Autopilot object is alive");
 
-            autopilot.Terminate(Autopilot.ExitCode.Normally);
+            await autopilot.TerminateAsync(Autopilot.ExitCode.Normally);
         }
 
         [Test]
@@ -66,7 +66,7 @@ public async Task Stop_TerminateAutopilotAndKeepPlayMode()
             var state = AutopilotState.Instance;
             editor.Launch(state);
             await Task.Delay(2000);
-            editor.Stop(); // Note: If Autopilot stops for life before Stop, a NullReference exception is raised here.
+            await editor.Stop(); // Note: If Autopilot stops for life before Stop, a NullReference exception is raised here.
 
             Assert.That(state.IsRunning, Is.False, "AutopilotState is terminated");
             Assert.That(EditorApplication.isPlaying, Is.True, "Keep play mode");

diff --git a/Tests/Runtime/TestDoubles/StubClickAgent.cs b/Tests/Runtime/TestDoubles/StubClickAgent.cs
@@ -0,0 +1,48 @@
+// Copyright (c) 2023 DeNA Co., Ltd.
+// This software is released under the MIT License.
+
+using System.Threading;
+using Cysharp.Threading.Tasks;
+using DeNA.Anjin.Agents;
+using UnityEngine;
+using UnityEngine.EventSystems;
+
+namespace DeNA.Anjin.TestDoubles
+{
+    /// <summary>
+    /// A test double for agent. This agent immediately click game objects that have the name specified
+    /// </summary>
+    public class StubClickAgent : AbstractAgent
+    {
+        /// <summary>
+        /// A name of game objects to click
+        /// </summary>
+        [SerializeField] public string targetName;
+
+
+        /// <inheritdoc />
+        public override UniTask Run(CancellationToken token)
+        {
+            foreach (var obj in FindObjectsByType<GameObject>(FindObjectsSortMode.None))
+            {
+                if (obj.name != targetName)
+                {
+                    continue;
+                }
+
+                var target = obj;
+                if (!target.TryGetComponent<IPointerClickHandler>(out var handler))
+                {
+                    Debug.LogWarning(
+                        $"{target.name} did not have any components that derived {nameof(IPointerClickHandler)}"
+                    );
+                    continue;
+                }
+
+                handler.OnPointerClick(new PointerEventData(EventSystem.current));
+            }
+
+            return UniTask.CompletedTask;
+        }
+    }
+}
diff --git a/Tests/Runtime/TestDoubles/StubClickAgent.cs.meta b/Tests/Runtime/TestDoubles/StubClickAgent.cs.meta
diff --git a/Tests/Runtime/TestDoubles/StubThrowingErrorOnClick.cs b/Tests/Runtime/TestDoubles/StubThrowingErrorOnClick.cs
@@ -0,0 +1,22 @@
+// Copyright (c) 2023 DeNA Co., Ltd.
+// This software is released under the MIT License.
+
+using System;
+using System.Threading;
+using UnityEngine;
+using UnityEngine.EventSystems;
+
+namespace DeNA.Anjin.TestDoubles
+{
+    /// <summary>
+    /// A test stub throw errors when clicked
+    /// </summary>
+    [AddComponentMenu("")]
+    public class StubThrowingErrorOnClick : MonoBehaviour, IPointerClickHandler
+    {
+        public void OnPointerClick(PointerEventData eventData)
+        {
+            throw new Exception($"TEST on thread #{Thread.CurrentThread.ManagedThreadId}");
+        }
+    }
+}
diff --git a/Tests/Runtime/TestDoubles/StubThrowingErrorOnClick.cs.meta b/Tests/Runtime/TestDoubles/StubThrowingErrorOnClick.cs.meta
diff --git a/Tests/TestAssets/AutopilotSettingsForFacingImmediatelyError.asset b/Tests/TestAssets/AutopilotSettingsForFacingImmediatelyError.asset
@@ -0,0 +1,33 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!114 &11400000
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: c7a1731caa574ed4b9334b7d52febe27, type: 3}
+  m_Name: AutopilotSettingsForFacingImmediatelyError
+  m_EditorClassIdentifier: 
+  description: 
+  sceneAgentMaps:
+  - scenePath: Packages/com.dena.anjin/Tests/TestScenes/Error.unity
+    agent: {fileID: 11400000, guid: 9a5c6d18335194c1b9f8e162695d73fd, type: 2}
+  fallbackAgent: {fileID: 0}
+  observerAgent: {fileID: 0}
+  lifespanSec: 0
+  randomSeed: 
+  timeScale: 1
+  junitReportPath: 
+  slackToken: 
+  slackChannels: 
+  mentionSubTeamIDs: 
+  addHereInSlackMessage: 0
+  handleException: 1
+  handleError: 1
+  handleAssert: 1
+  handleWarning: 0
+  ignoreMessages: []
diff --git a/Tests/TestAssets/AutopilotSettingsForFacingImmediatelyError.asset.meta b/Tests/TestAssets/AutopilotSettingsForFacingImmediatelyError.asset.meta
diff --git a/Tests/TestAssets/StubClickAgentForTests.asset b/Tests/TestAssets/StubClickAgentForTests.asset
@@ -0,0 +1,16 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!114 &11400000
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: fcc96872a9564f89be6b16a814ad0034, type: 3}
+  m_Name: StubClickAgentForTests
+  m_EditorClassIdentifier: 
+  description: 
+  targetName: ThrowErrorFromMainThreadOnClick
diff --git a/Tests/TestAssets/StubClickAgentForTests.asset.meta b/Tests/TestAssets/StubClickAgentForTests.asset.meta
diff --git a/Tests/TestAssets/StubImmediatelyErrorAgentForTests.asset b/Tests/TestAssets/StubImmediatelyErrorAgentForTests.asset
@@ -0,0 +1,15 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!114 &11400000
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: bc82cbd7e5b6404da291e090ed1f4eba, type: 3}
+  m_Name: StubImmediatelyErrorAgentForTests
+  m_EditorClassIdentifier: 
+  description: 
diff --git a/Tests/TestAssets/StubImmediatelyErrorAgentForTests.asset.meta b/Tests/TestAssets/StubImmediatelyErrorAgentForTests.asset.meta