Fix exception handling during session creation

Fix that exceptions during session creation could leave the app process
still running.

Author: aristotelos

src/FlaUI.WebDriver/Controllers/SessionController.cs

@@ -1,3 +1,4 @@
+using FlaUI.Core;
 using FlaUI.WebDriver.Models;
 using Microsoft.AspNetCore.Mvc;
 using System.Diagnostics;
@@ -29,8 +30,6 @@ public async Task<ActionResult> CreateNewSession([FromBody] CreateSessionRequest
                 .Where(capabilities => TryMatchCapabilities(capabilities, out matchedCapabilities, out _))
                 .Select(capabillities => matchedCapabilities!);
 
-            Core.Application? app;
-            var isAppOwnedBySession = false;
             var capabilities = matchingCapabilities.FirstOrDefault();
             if (capabilities == null)
             {
@@ -43,69 +42,87 @@ public async Task<ActionResult> CreateNewSession([FromBody] CreateSessionRequest
                     Message = string.Join("; ", mismatchIndications)
                 });
             }
+
+            var timeoutsConfiguration = new TimeoutsConfiguration();
+            if (capabilities.TryGetCapability<TimeoutsConfiguration>("timeouts", out var timeoutsConfigurationFromCapabilities))
+            {
+                timeoutsConfiguration = timeoutsConfigurationFromCapabilities!;
+            }
+            var newCommandTimeout = TimeSpan.FromSeconds(60);
+            if (capabilities.TryGetNumberCapability("appium:newCommandTimeout", out var newCommandTimeoutFromCapabilities))
+            {
+                newCommandTimeout = TimeSpan.FromSeconds(newCommandTimeoutFromCapabilities);
+            }
+
+            Application? app = GetApp(capabilities, out bool isAppOwnedBySession);
+            Session session;
+            try
+            {
+                session = new Session(app, isAppOwnedBySession, timeoutsConfiguration, newCommandTimeout);
+            }
+            catch (Exception)
+            {
+                app?.Dispose();
+                throw;
+            }
+
+            _sessionRepository.Add(session);
+            _logger.LogInformation("Created session with ID {SessionId} and capabilities {Capabilities}", session.SessionId, capabilities);
+            return await Task.FromResult(WebDriverResult.Success(new CreateSessionResponse()
+            {
+                SessionId = session.SessionId,
+                Capabilities = capabilities.Capabilities
+            }));
+        }
+
+        private static Application? GetApp(MergedCapabilities capabilities, out bool isAppOwnedBySession)
+        {
             if (capabilities.TryGetStringCapability("appium:app", out var appPath))
             {
                 if (appPath == "Root")
                 {
-                    app = null;
+                    isAppOwnedBySession = false;
+                    return null;
                 }
-                else 
+
+                capabilities.TryGetStringCapability("appium:appArguments", out var appArguments);
+                try
                 {
-                    capabilities.TryGetStringCapability("appium:appArguments", out var appArguments);
-                    try
+                    if (appPath.EndsWith("!App"))
                     {
-                        if (appPath.EndsWith("!App"))
-                        {
-                            app = Core.Application.LaunchStoreApp(appPath, appArguments);
-                        }
-                        else
-                        {
-                            var processStartInfo = new ProcessStartInfo(appPath, appArguments ?? "");
-                            if(capabilities.TryGetStringCapability("appium:appWorkingDir", out var appWorkingDir))
-                            {
-                                processStartInfo.WorkingDirectory = appWorkingDir;
-                            }
-                            app = Core.Application.Launch(processStartInfo);
-                        }
+                        isAppOwnedBySession = true;
+                        return Application.LaunchStoreApp(appPath, appArguments);
                     }
-                    catch(Exception e)
+
+                    var processStartInfo = new ProcessStartInfo(appPath, appArguments ?? "");
+                    if (capabilities.TryGetStringCapability("appium:appWorkingDir", out var appWorkingDir))
                     {
-                        throw WebDriverResponseException.InvalidArgument($"Starting app '{appPath}' with arguments '{appArguments}' threw an exception: {e.Message}");
+                        processStartInfo.WorkingDirectory = appWorkingDir;
                     }
+                    isAppOwnedBySession = true;
+                    return Application.Launch(processStartInfo);
+                }
+                catch (Exception e)
+                {
+                    throw WebDriverResponseException.InvalidArgument($"Starting app '{appPath}' with arguments '{appArguments}' threw an exception: {e.Message}");
                 }
-
-                isAppOwnedBySession = true;
             }
-            else if (capabilities.TryGetStringCapability("appium:appTopLevelWindow", out var appTopLevelWindowString))
+
+            if (capabilities.TryGetStringCapability("appium:appTopLevelWindow", out var appTopLevelWindowString))
             {
+                isAppOwnedBySession = false;
                 Process process = GetProcessByMainWindowHandle(appTopLevelWindowString);
-                app = Core.Application.Attach(process);
+                return Application.Attach(process);
             }
-            else if (capabilities.TryGetStringCapability("appium:appTopLevelWindowTitleMatch", out var appTopLevelWindowTitleMatch))
+
+            if (capabilities.TryGetStringCapability("appium:appTopLevelWindowTitleMatch", out var appTopLevelWindowTitleMatch))
             {
+                isAppOwnedBySession = false;
                 Process? process = GetProcessByMainWindowTitle(appTopLevelWindowTitleMatch);
-                app = Core.Application.Attach(process);
-            }
-            else
-            {
-                throw WebDriverResponseException.InvalidArgument("One of appium:app, appium:appTopLevelWindow or appium:appTopLevelWindowTitleMatch must be passed as a capability");
-            }
-            var session = new Session(app, isAppOwnedBySession);
-            if(capabilities.TryGetNumberCapability("appium:newCommandTimeout", out var newCommandTimeout))
-            {
-                session.NewCommandTimeout = TimeSpan.FromSeconds(newCommandTimeout);
+                return Application.Attach(process);
             }
-            if (capabilities.TryGetCapability<TimeoutsConfiguration>("timeouts", out var timeoutsConfiguration))
-            {
-                session.TimeoutsConfiguration = timeoutsConfiguration!;
-            }
-            _sessionRepository.Add(session);
-            _logger.LogInformation("Created session with ID {SessionId} and capabilities {Capabilities}", session.SessionId, capabilities);
-            return await Task.FromResult(WebDriverResult.Success(new CreateSessionResponse()
-            {
-                SessionId = session.SessionId,
-                Capabilities = capabilities.Capabilities
-            }));
+
+            throw WebDriverResponseException.InvalidArgument("One of appium:app, appium:appTopLevelWindow or appium:appTopLevelWindowTitleMatch must be passed as a capability");
         }
 
         private string? GetMismatchIndication(MergedCapabilities capabilities)
@@ -163,7 +180,7 @@ private bool TryMatchCapabilities(MergedCapabilities capabilities, [MaybeNullWhe
                 matchedCapabilities.Copy("appium:appTopLevelWindow", capabilities);
             }
             else if (capabilities.Contains("appium:appTopLevelWindowTitleMatch"))
-            {   
+            {
                 matchedCapabilities.Copy("appium:appTopLevelWindowTitleMatch", capabilities);
             }
             else
@@ -199,8 +216,8 @@ private static Process GetProcessByMainWindowTitle(string appTopLevelWindowTitle
             try
             {
                 appMainWindowTitleRegex = new Regex(appTopLevelWindowTitleMatch);
-            } 
-            catch(ArgumentException e)
+            }
+            catch (ArgumentException e)
             {
                 throw WebDriverResponseException.InvalidArgument($"Capability appium:appTopLevelWindowTitleMatch '{appTopLevelWindowTitleMatch}' is not a valid regular expression: {e.Message}");
             }
@@ -209,7 +226,7 @@ private static Process GetProcessByMainWindowTitle(string appTopLevelWindowTitle
             {
                 throw WebDriverResponseException.InvalidArgument($"Process with main window title matching '{appTopLevelWindowTitleMatch}' could not be found");
             }
-            else if (processes.Length > 1)
+            if (processes.Length > 1)
             {
                 throw WebDriverResponseException.InvalidArgument($"Found multiple ({processes.Length}) processes with main window title matching '{appTopLevelWindowTitleMatch}'");
             }

src/FlaUI.WebDriver/Session.cs

@@ -8,14 +8,15 @@ namespace FlaUI.WebDriver
 {
     public class Session : IDisposable
     {
-        public Session(Application? app, bool isAppOwnedBySession)
+        public Session(Application? app, bool isAppOwnedBySession, TimeoutsConfiguration timeoutsConfiguration, TimeSpan newCommandTimeout)
         {
             App = app;
             SessionId = Guid.NewGuid().ToString();
             Automation = new UIA3Automation();
             InputState = new InputState();
-            TimeoutsConfiguration = new TimeoutsConfiguration();
+            TimeoutsConfiguration = timeoutsConfiguration;
             IsAppOwnedBySession = isAppOwnedBySession;
+            NewCommandTimeout = newCommandTimeout;
 
             if (app != null)
             {