From fecd92879be485c3585a536f3eaaeda2fd59f942 Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Tue, 12 Nov 2024 15:20:44 +0100 Subject: [PATCH 1/4] Fix ignored exceptions in MqttHostedServer startup --- Source/MQTTnet.AspnetCore/MqttHostedServer.cs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs index 4c74f6a43..c3aa1f316 100644 --- a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs +++ b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs @@ -7,6 +7,8 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using MQTTnet.Diagnostics.Logger; using MQTTnet.Server; @@ -16,16 +18,20 @@ public sealed class MqttHostedServer : MqttServer, IHostedService { readonly IHostApplicationLifetime _hostApplicationLifetime; readonly MqttServerFactory _mqttFactory; + readonly ILogger _appLogger; public MqttHostedServer( IHostApplicationLifetime hostApplicationLifetime, MqttServerFactory mqttFactory, MqttServerOptions options, IEnumerable adapters, - IMqttNetLogger logger) : base(options, adapters, logger) + IMqttNetLogger logger, + ILogger appLogger = null + ) : base(options, adapters, logger) { _mqttFactory = mqttFactory ?? throw new ArgumentNullException(nameof(mqttFactory)); _hostApplicationLifetime = hostApplicationLifetime; + _appLogger = appLogger ?? NullLogger.Instance; } public async Task StartAsync(CancellationToken cancellationToken) @@ -43,6 +49,19 @@ public Task StopAsync(CancellationToken cancellationToken) void OnStarted() { - _ = StartAsync(); + async Task DoStart() + { + try + { + await StartAsync(); + } + catch (Exception e) + { + _appLogger.LogError(e, "Stopping application: failed to start MqttServer: {Error}", e.Message); + _hostApplicationLifetime.StopApplication(); + throw; + } + } + _ = DoStart(); } } \ No newline at end of file From 4eb737f32134e8d5b21287b21e3b412952bcc7b4 Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Thu, 14 Nov 2024 11:56:05 +0100 Subject: [PATCH 2/4] MqttHostedServer based on BackgroundService --- Source/MQTTnet.AspnetCore/MqttHostedServer.cs | 50 ++++--------------- .../ServiceCollectionExtensions.cs | 4 +- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs index c3aa1f316..e560fdb85 100644 --- a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs +++ b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs @@ -7,61 +7,33 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; using MQTTnet.Diagnostics.Logger; using MQTTnet.Server; namespace MQTTnet.AspNetCore; -public sealed class MqttHostedServer : MqttServer, IHostedService +public sealed class MqttHostedServer : BackgroundService { - readonly IHostApplicationLifetime _hostApplicationLifetime; readonly MqttServerFactory _mqttFactory; - readonly ILogger _appLogger; - public MqttHostedServer( - IHostApplicationLifetime hostApplicationLifetime, MqttServerFactory mqttFactory, MqttServerOptions options, IEnumerable adapters, - IMqttNetLogger logger, - ILogger appLogger = null - ) : base(options, adapters, logger) + IMqttNetLogger logger + ) { + MqttServer = new(options, adapters, logger); _mqttFactory = mqttFactory ?? throw new ArgumentNullException(nameof(mqttFactory)); - _hostApplicationLifetime = hostApplicationLifetime; - _appLogger = appLogger ?? NullLogger.Instance; - } - - public async Task StartAsync(CancellationToken cancellationToken) - { - // The yield makes sure that the hosted service is considered up and running. - await Task.Yield(); - - _hostApplicationLifetime.ApplicationStarted.Register(OnStarted); } - public Task StopAsync(CancellationToken cancellationToken) + public MqttServer MqttServer { get; } + protected override async Task ExecuteAsync(CancellationToken stoppingToken) { - return StopAsync(_mqttFactory.CreateMqttServerStopOptionsBuilder().Build()); + await MqttServer.StartAsync(); } - - void OnStarted() + public override async Task StopAsync(CancellationToken cancellationToken) { - async Task DoStart() - { - try - { - await StartAsync(); - } - catch (Exception e) - { - _appLogger.LogError(e, "Stopping application: failed to start MqttServer: {Error}", e.Message); - _hostApplicationLifetime.StopApplication(); - throw; - } - } - _ = DoStart(); + await MqttServer.StopAsync(_mqttFactory.CreateMqttServerStopOptionsBuilder().Build()); + await base.StopAsync(cancellationToken); } -} \ No newline at end of file +} diff --git a/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs b/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs index 915f6791c..2f03d0334 100644 --- a/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs +++ b/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs @@ -45,8 +45,8 @@ public static void AddHostedMqttServer(this IServiceCollection services) services.TryAddSingleton(new MqttServerFactory()); services.AddSingleton(); - services.AddSingleton(s => s.GetService()); - services.AddSingleton(s => s.GetService()); + services.AddHostedService(s => s.GetService()); + services.AddSingleton(s => s.GetService().MqttServer); } public static IServiceCollection AddHostedMqttServerWithServices(this IServiceCollection services, Action configure) From 672435c37a8a12975f7b6e140891db1e6af91fce Mon Sep 17 00:00:00 2001 From: beppemarazzi Date: Thu, 14 Nov 2024 12:02:21 +0100 Subject: [PATCH 3/4] minor refactoring --- Source/MQTTnet.AspnetCore/MqttHostedServer.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs index e560fdb85..3dfd6c648 100644 --- a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs +++ b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs @@ -27,10 +27,8 @@ IMqttNetLogger logger } public MqttServer MqttServer { get; } - protected override async Task ExecuteAsync(CancellationToken stoppingToken) - { - await MqttServer.StartAsync(); - } + protected override Task ExecuteAsync(CancellationToken stoppingToken) + => MqttServer.StartAsync(); public override async Task StopAsync(CancellationToken cancellationToken) { await MqttServer.StopAsync(_mqttFactory.CreateMqttServerStopOptionsBuilder().Build()); From 9466cb70d574ed5b95dd833cd47f94158197cd64 Mon Sep 17 00:00:00 2001 From: Giuseppe Marazzi Date: Mon, 6 Jan 2025 11:18:55 +0100 Subject: [PATCH 4/4] Add Test MqttHostedServerStartup --- .../MQTTnet.AspNetCore.csproj | 5 ++ Source/MQTTnet.AspnetCore/MqttHostedServer.cs | 55 ++++++++++++++++ .../ServiceCollectionExtensions.cs | 8 +++ .../ASP/MqttHostedServerStartup.cs | 66 +++++++++++++++++++ 4 files changed, 134 insertions(+) create mode 100644 Source/MQTTnet.Tests/ASP/MqttHostedServerStartup.cs diff --git a/Source/MQTTnet.AspnetCore/MQTTnet.AspNetCore.csproj b/Source/MQTTnet.AspnetCore/MQTTnet.AspNetCore.csproj index 5357dd702..022747b51 100644 --- a/Source/MQTTnet.AspnetCore/MQTTnet.AspNetCore.csproj +++ b/Source/MQTTnet.AspnetCore/MQTTnet.AspNetCore.csproj @@ -1,5 +1,10 @@ + + + $(DefineConstants);HOSTEDSERVER_WITHOUT_STARTUP_YIELD + + net8.0 MQTTnet.AspNetCore diff --git a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs index 3dfd6c648..69ce0b39a 100644 --- a/Source/MQTTnet.AspnetCore/MqttHostedServer.cs +++ b/Source/MQTTnet.AspnetCore/MqttHostedServer.cs @@ -1,3 +1,56 @@ +//TODO: remove before close https://github.com/dotnet/MQTTnet/pull/2102 +////WIP: Fix ignored exceptions in MqttHostedServer startup #2102 +#if !HOSTEDSERVER_WITHOUT_STARTUP_YIELD +// 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 System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Hosting; +using MQTTnet.Diagnostics.Logger; +using MQTTnet.Server; + +namespace MQTTnet.AspNetCore; + +public sealed class MqttHostedServer : MqttServer, IHostedService +{ + readonly IHostApplicationLifetime _hostApplicationLifetime; + readonly MqttServerFactory _mqttFactory; + + public MqttHostedServer( + IHostApplicationLifetime hostApplicationLifetime, + MqttServerFactory mqttFactory, + MqttServerOptions options, + IEnumerable adapters, + IMqttNetLogger logger) : base(options, adapters, logger) + { + _mqttFactory = mqttFactory ?? throw new ArgumentNullException(nameof(mqttFactory)); + _hostApplicationLifetime = hostApplicationLifetime; + } + + public async Task StartAsync(CancellationToken cancellationToken) + { + // The yield makes sure that the hosted service is considered up and running. + await Task.Yield(); + + _hostApplicationLifetime.ApplicationStarted.Register(OnStarted); + } + + public Task StopAsync(CancellationToken cancellationToken) + { + return StopAsync(_mqttFactory.CreateMqttServerStopOptionsBuilder().Build()); + } + + void OnStarted() + { + _ = StartAsync(); + } +} + +#else // 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. @@ -35,3 +88,5 @@ public override async Task StopAsync(CancellationToken cancellationToken) await base.StopAsync(cancellationToken); } } + +#endif \ No newline at end of file diff --git a/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs b/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs index 2f03d0334..6f889ae7c 100644 --- a/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs +++ b/Source/MQTTnet.AspnetCore/ServiceCollectionExtensions.cs @@ -45,8 +45,16 @@ public static void AddHostedMqttServer(this IServiceCollection services) services.TryAddSingleton(new MqttServerFactory()); services.AddSingleton(); + //TODO: remove before close https://github.com/dotnet/MQTTnet/pull/2102 + ////WIP: Fix ignored exceptions in MqttHostedServer startup #2102 +#if !HOSTEDSERVER_WITHOUT_STARTUP_YIELD + services.AddSingleton(s => s.GetService()); + services.AddSingleton(s => s.GetService()); +#else services.AddHostedService(s => s.GetService()); services.AddSingleton(s => s.GetService().MqttServer); +#endif + } public static IServiceCollection AddHostedMqttServerWithServices(this IServiceCollection services, Action configure) diff --git a/Source/MQTTnet.Tests/ASP/MqttHostedServerStartup.cs b/Source/MQTTnet.Tests/ASP/MqttHostedServerStartup.cs new file mode 100644 index 000000000..d3b1e9cd5 --- /dev/null +++ b/Source/MQTTnet.Tests/ASP/MqttHostedServerStartup.cs @@ -0,0 +1,66 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using MQTTnet.AspNetCore; +using System.Net.Sockets; +using System.Net; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using MQTTnet.Server; +using Microsoft.AspNetCore.Hosting; + +namespace MQTTnet.Tests.ASP +{ + [TestClass] + public class MqttHostedServerStartup + { + private async Task TestStartup(bool useOccupiedPort) + { + using TcpListener l = new TcpListener(IPAddress.Any, 0); + l.Start(); + int port = ((IPEndPoint)l.LocalEndpoint).Port; + + if(!useOccupiedPort) + l.Stop(); + + + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseUrls("http://127.0.0.1:0"); + + builder.Services.AddMqttTcpServerAdapter(); + builder.Services.AddHostedMqttServer(cfg => + { + cfg + .WithDefaultEndpoint() + .WithDefaultEndpointPort(port); + }); + + + var app = builder.Build(); + + if(!useOccupiedPort) + { + await app.StartAsync(); + var server = app.Services.GetRequiredService(); + Assert.IsTrue(server.IsStarted); + await app.StopAsync(); + } + else + { + await Assert.ThrowsExceptionAsync(() => + app.StartAsync() + ); + } + } + + [TestMethod] + [DoNotParallelize] + public Task TestSuccessfullyStartup() + => TestStartup(useOccupiedPort: false); + + [TestMethod] + [DoNotParallelize] + public Task TestFailedStartup() + => TestStartup(useOccupiedPort: true); + + } +}