Improve port conflict handling for proxy and API ports

DevProxy.Proxy.Kestrel/KestrelProxyEngine.cs

@@ -35,7 +35,44 @@ public sealed class KestrelProxyEngine(
 {
     private readonly ILogger _logger = loggerFactory.CreateLogger<KestrelProxyEngine>();
 
+    /// <summary>
+    /// True when the engine stopped the host because the proxy port was already in use.
+    /// </summary>
+    public bool PortInUse { get; private set; }
+
+    private readonly TaskCompletionSource<bool> _started = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+    /// <summary>
+    /// Completes with true once the proxy is listening, or with false if it
+    /// stopped before it could start listening (for example, port in use).
+    /// </summary>
+    public Task<bool> Started => _started.Task;
+
+    private static bool IsAddressInUse(Exception? ex)
+    {
+        for (var current = ex; current is not null; current = current.InnerException)
+        {
+            if (current is AddressInUseException)
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
     protected override async Task ExecuteAsync(CancellationToken stoppingToken)
+    {
+        try
+        {
+            await RunAsync(stoppingToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            _ = _started.TrySetResult(false);
+        }
+    }
+
+    private async Task RunAsync(CancellationToken stoppingToken)
     {
         var ipAddress = string.IsNullOrWhiteSpace(configuration.IPAddress)
             ? IPAddress.Loopback
@@ -94,7 +131,20 @@ configuration is IProcessTreeProxyConfiguration processTreeConfiguration &&
 
         await using var app = builder.Build();
 
-        await app.StartAsync(stoppingToken).ConfigureAwait(false);
+        try
+        {
+            await app.StartAsync(stoppingToken).ConfigureAwait(false);
+        }
+        catch (IOException ex) when (hostApplicationLifetime is not null && IsAddressInUse(ex))
+        {
+            // Let the host report a friendly message instead of crashing with a stack trace.
+            PortInUse = true;
+            _logger.LogDebug(ex, "Failed to bind the proxy to {Address}:{Port}", ipAddress, port);
+            // Stopping while the host is still starting surfaces as a cancellation
+            // instead of a clean shutdown, so wait until it has started.
+            _ = hostApplicationLifetime.ApplicationStarted.Register(hostApplicationLifetime.StopApplication);
+            return;
+        }
         inactivityTimer?.Reset();
         // When --port 0 is used the OS assigns a free port; Kestrel rewrites the
         // ListenOptions endpoint to the bound port after StartAsync, so log THAT
@@ -118,6 +168,8 @@ configuration is IProcessTreeProxyConfiguration processTreeConfiguration &&
             systemProxyEnabled = true;
         }
 
+        _ = _started.TrySetResult(true);
+
         try
         {
             await Task.Delay(Timeout.Infinite, stoppingToken).ConfigureAwait(false);

DevProxy.Tests/PortConflictTests.cs

@@ -0,0 +1,166 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using DevProxy.Commands;
+using DevProxy.Proxy;
+using DevProxy.State;
+using Microsoft.AspNetCore.Connections;
+using System.CommandLine;
+using System.Net;
+using System.Net.Sockets;
+using Xunit;
+
+namespace DevProxy.Tests;
+
+public class PortConflictTests
+{
+    [Fact]
+    public void IsPortAvailable_ReturnsFalse_WhenPortIsInUse()
+    {
+        using var listener = new TcpListener(IPAddress.Loopback, 0);
+        listener.Start();
+        var port = ((IPEndPoint)listener.LocalEndpoint).Port;
+
+        Assert.False(PortConflict.IsPortAvailable("127.0.0.1", port));
+    }
+
+    [Fact]
+    public void IsPortAvailable_ReturnsTrue_WhenPortIsFree()
+    {
+        int port;
+        using (var listener = new TcpListener(IPAddress.Loopback, 0))
+        {
+            listener.Start();
+            port = ((IPEndPoint)listener.LocalEndpoint).Port;
+        }
+
+        Assert.True(PortConflict.IsPortAvailable("127.0.0.1", port));
+    }
+
+    [Theory]
+    [InlineData("127.0.0.1", 0)]
+    [InlineData("127.0.0.1", 65536)]
+    [InlineData("not-an-ip", 8000)]
+    public void IsPortAvailable_LeavesUncheckableInputToKestrel(string ipAddress, int port)
+    {
+        Assert.True(PortConflict.IsPortAvailable(ipAddress, port));
+    }
+
+    private static ProxyInstanceState State(int pid, string proxyUrl, int port, string apiUrl) =>
+        new() { Pid = pid, ProxyUrl = proxyUrl, Port = port, ApiUrl = apiUrl };
+
+    [Fact]
+    public void FindOwningInstance_MatchesProxyPort()
+    {
+        var states = new[]
+        {
+            State(1, "http://127.0.0.1:8001", 8001, "http://127.0.0.1:8898/"),
+            State(2, "http://127.0.0.1:8000", 8000, "http://127.0.0.1:8897/")
+        };
+
+        Assert.Equal(2, PortConflict.FindOwningInstance(states, "127.0.0.1", 8000, excludePid: 0)?.Pid);
+    }
+
+    [Fact]
+    public void FindOwningInstance_MatchesApiPort()
+    {
+        var states = new[]
+        {
+            State(1, "http://127.0.0.1:8001", 8001, "http://127.0.0.1:8898/"),
+            State(2, "http://127.0.0.1:8000", 8000, "http://127.0.0.1:8897")
+        };
+
+        Assert.Equal(2, PortConflict.FindOwningInstance(states, "127.0.0.1", 8897, excludePid: 0)?.Pid);
+    }
+
+    [Fact]
+    public void FindOwningInstance_MatchesAcrossPortKinds()
+    {
+        // Another instance's API listener can hold the port requested for the proxy, and vice versa.
+        var states = new[] { State(2, "http://127.0.0.1:8000", 8000, "http://127.0.0.1:8897") };
+
+        Assert.Equal(2, PortConflict.FindOwningInstance(states, "127.0.0.1", 8897, excludePid: 0)?.Pid);
+        Assert.Equal(2, PortConflict.FindOwningInstance(states, null, 8000, excludePid: 0)?.Pid);
+    }
+
+    [Theory]
+    [InlineData("http://127.0.0.1:8000", "127.0.0.1", true)]
+    [InlineData("http://0.0.0.0:8000", "127.0.0.1", true)]
+    [InlineData("http://127.0.0.1:8000", "0.0.0.0", true)]
+    [InlineData("http://[::]:8000", "::1", true)]
+    [InlineData("http://[::1]:8000", "[::1]", true)]
+    [InlineData("http://127.0.0.1:8000", "192.168.1.10", false)]
+    [InlineData("http://[::1]:8000", "127.0.0.1", false)]
+    [InlineData("http://localhost:8000", "127.0.0.1", false)]
+    [InlineData("http://127.0.0.1:8000", "not-an-ip", false)]
+    [InlineData("http://0.0.0.0:8000", "[::1]", false)]
+    [InlineData("http://[::1]:8000", "0.0.0.0", false)]
+    [InlineData("http://[::]:8000", "127.0.0.1", true)]
+    [InlineData("http://127.0.0.1:8000", "::", true)]
+    public void FindOwningInstance_RequiresOverlappingAddress(string proxyUrl, string requestedAddress, bool expectedMatch)
+    {
+        var states = new[] { State(2, proxyUrl, 8000, "http://127.0.0.1:8897") };
+
+        var owner = PortConflict.FindOwningInstance(states, requestedAddress, 8000, excludePid: 0);
+
+        Assert.Equal(expectedMatch, owner is not null);
+    }
+
+    [Fact]
+    public void FindOwningInstance_IgnoresCurrentProcessAndPortZero()
+    {
+        var states = new[] { State(5, "http://127.0.0.1:8000", 8000, "http://127.0.0.1:0") };
+
+        Assert.Null(PortConflict.FindOwningInstance(states, "127.0.0.1", 8000, excludePid: 5));
+        Assert.Null(PortConflict.FindOwningInstance(states, "127.0.0.1", 0, excludePid: 0));
+    }
+
+    [Fact]
+    public void FormatMessage_DevProxyInstance_SuggestsStopAndPortZero()
+    {
+        var message = PortConflict.FormatMessage(PortKind.Api, 8897, new ProxyInstanceState { Pid = 42 });
+
+        Assert.Contains("API port 8897 is already in use by another Dev Proxy instance (PID: 42).", message, StringComparison.Ordinal);
+        Assert.Contains("devproxy stop --pid 42", message, StringComparison.Ordinal);
+        Assert.Contains("devproxy --api-port 0", message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public void FormatMessage_OtherApplication_SuggestsPortZeroAndConfig()
+    {
+        var message = PortConflict.FormatMessage(PortKind.Proxy, 8000, owner: null);
+
+        Assert.Contains("Port 8000 is already in use by another application.", message, StringComparison.Ordinal);
+        Assert.Contains("devproxy --port 0", message, StringComparison.Ordinal);
+        Assert.Contains("'port' in your devproxyrc.json", message, StringComparison.Ordinal);
+        Assert.DoesNotContain("devproxy stop", message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public void IsAddressInUse_DetectsWrappedKestrelException()
+    {
+        var ex = new IOException("Failed to bind", new AddressInUseException("Address already in use"));
+
+        Assert.True(PortConflict.IsAddressInUse(ex));
+        Assert.False(PortConflict.IsAddressInUse(new IOException("other")));
+    }
+
+    [Theory]
+    [InlineData("0", true)]
+    [InlineData("8000", true)]
+    [InlineData("65535", true)]
+    [InlineData("65536", false)]
+    [InlineData("-1", false)]
+    [InlineData("abc", false)]
+    public void PortOptions_ValidateRange(string value, bool valid)
+    {
+        var option = new Option<int?>(DevProxyCommand.PortOptionName);
+        option.Validators.Add(DevProxyCommand.ValidatePort);
+        var command = new RootCommand { option };
+
+        var result = command.Parse([DevProxyCommand.PortOptionName, value]);
+
+        Assert.Equal(valid, result.Errors.Count == 0);
+    }
+}

DevProxy/Commands/DevProxyCommand.cs

@@ -2,6 +2,7 @@
 using DevProxy.Abstractions.Proxy;
 using DevProxy.Abstractions.Utils;
 using DevProxy.Proxy;
+using DevProxy.Proxy.Kestrel;
 using DevProxy.State;
 using Microsoft.AspNetCore.Hosting.Server;
 using Microsoft.AspNetCore.Hosting.Server.Features;
@@ -278,6 +279,15 @@ public async Task<int> InvokeAsync(string[] args, WebApplication app)
         return exitCode;
     }
 
+    internal static void ValidatePort(OptionResult input)
+    {
+        var value = input.Tokens.Count > 0 ? input.Tokens[0].Value : null;
+        if (!int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out var port) || port > System.Net.IPEndPoint.MaxPort)
+        {
+            input.AddError($"'{value}' is not a valid port. Specify a number between 0 and {System.Net.IPEndPoint.MaxPort}.");
+        }
+    }
+
     private async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
     {
         if (_app is null)
@@ -303,6 +313,16 @@ private async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken c
         }
 
         ConfigureFromOptions(parseResult);
+
+        var portConflict =
+            await PortConflict.CheckAsync(PortKind.Proxy, _proxyConfiguration.IPAddress, _proxyConfiguration.Port, cancellationToken) ??
+            await PortConflict.CheckAsync(PortKind.Api, _proxyConfiguration.ApiIpAddress, _proxyConfiguration.ApiPort, cancellationToken);
+        if (portConflict is not null)
+        {
+            _logger.LogError("{PortConflict}", portConflict);
+            return 1;
+        }
+
         var optionsLoadedArgs = new OptionsLoadedArgs(parseResult);
         foreach (var plugin in _plugins.Where(p => p.Enabled))
         {
@@ -316,6 +336,25 @@ private async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken c
             await ApiSecurity.SaveTokenAsync(cancellationToken);
             await _app.StartAsync(cancellationToken);
 
+            // The proxy binds in a background service, so wait for it before
+            // publishing readiness. Otherwise a detached launcher could report a
+            // successful start for a proxy that failed to bind. Don't pass the
+            // cancellation token: a bind failure stops the host, which would cancel
+            // the wait before the failure is reported. The engine always completes
+            // Started once the host stops.
+            var proxyEngines = _app.Services.GetServices<IHostedService>().OfType<KestrelProxyEngine>().ToArray();
+            var proxyStarted = (await Task.WhenAll(proxyEngines.Select(e => e.Started))).All(started => started);
+            if (!proxyStarted)
+            {
+                await _app.WaitForShutdownAsync(cancellationToken);
+                if (proxyEngines.Any(e => e.PortInUse))
+                {
+                    _logger.LogError("{PortConflict}",
+                        await PortConflict.GetMessageAsync(PortKind.Proxy, _proxyConfiguration.IPAddress, _proxyConfiguration.Port, CancellationToken.None));
+                }
+                return 1;
+            }
+
             var serverAddresses = _app.Services.GetRequiredService<IServer>().Features.Get<IServerAddressesFeature>();
             var serverAddress = serverAddresses?.Addresses.FirstOrDefault();
             var address = ApiSecurity.GetApiUrl(serverAddress ??
@@ -344,6 +383,13 @@ private async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken c
         {
             throw;
         }
+        catch (Exception ex) when (PortConflict.IsAddressInUse(ex))
+        {
+            // The API port was taken between the pre-flight check and binding.
+            _logger.LogError("{PortConflict}",
+                await PortConflict.GetMessageAsync(PortKind.Api, _proxyConfiguration.ApiIpAddress, _proxyConfiguration.ApiPort, CancellationToken.None));
+            return 1;
+        }
         catch (Exception ex)
         {
             _logger.LogError(ex, "An error occurred while running Dev Proxy");
@@ -419,12 +465,14 @@ private void ConfigureCommand(DevProxyConfigOptions configOptions)
             Description = "The port for the proxy to listen on",
             HelpName = "port"
         };
+        portOption.Validators.Add(ValidatePort);
 
         var apiPortOption = new Option<int?>(ApiPortOptionName)
         {
             Description = "The port for the Dev Proxy API to listen on",
             HelpName = "api-port"
         };
+        apiPortOption.Validators.Add(ValidatePort);
         var apiIpAddressOption = new Option<string?>(ApiIpAddressOptionName)
         {
             Description = "The API bind address (default: 127.0.0.1). Use ::1 for IPv6 loopback. Non-loopback binding requires a trusted network",