From d4ec84d5fbb1e58dbc91f303fee254a86aee504f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 21:51:33 -0400 Subject: [PATCH] fix(inbound): log swallowed scope-creation failure + test scope disposal on script throw --- .../InboundScriptExecutor.cs | 9 +++- .../InboundScriptExecutorTests.cs | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs index 2a63d32b..75f1b1bc 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs @@ -251,9 +251,14 @@ public class InboundScriptExecutor { scope = _serviceProvider.CreateScope(); } - catch (InvalidOperationException) + catch (InvalidOperationException ex) { - // No scope factory available (provider does not support scoping). + // No scope factory available (e.g. a non-scoping test-double provider). + // In production this should never happen; log so a genuine container + // misconfiguration is visible rather than silently disabling Database. + _logger.LogWarning(ex, + "Could not create a DI scope for method {Method}; Database will be unavailable to its script", + method.Name); } try diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs index ccbec43f..bdfdde0b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs @@ -599,6 +599,51 @@ public class InboundScriptExecutorTests Assert.Contains("99", result.ResultJson!); } + [Fact] + public async Task ScriptThrows_DisposesDiScope() + { + // When the script handler throws, ExecuteAsync must still dispose the + // per-execution DI scope it created (regression guard for the finally block). + var scopeServiceProvider = Substitute.For(); + // GetService() returns null — script never needs it. + scopeServiceProvider.GetService(typeof(IDatabaseGateway)).Returns((object?)null); + + var scope = Substitute.For(); + scope.ServiceProvider.Returns(scopeServiceProvider); + + var factory = Substitute.For(); + factory.CreateScope().Returns(scope); + + // Wire the factory into the provider so CreateScope() extension finds it. + var provider = Substitute.For(); + provider.GetService(typeof(IServiceScopeFactory)).Returns(factory); + + var executor = new InboundScriptExecutor( + NullLogger.Instance, provider); + + // A pre-registered handler that throws; ReturnDefinition=null so validation + // is not the failure path. + var method = new ApiMethod("throws", "throw new Exception(\"boom\");") + { + Id = 1, + TimeoutSeconds = 10, + ReturnDefinition = null, + }; + executor.RegisterHandler("throws", _ => throw new Exception("boom")); + + var result = await executor.ExecuteAsync( + method, + new Dictionary(), + _route, + TimeSpan.FromSeconds(10)); + + // The executor must swallow the script exception and return a non-success result. + Assert.False(result.Success); + + // The scope must have been disposed via the finally block. + scope.Received(1).Dispose(); + } + // SQLite-backed IDatabaseGateway fake (mirrors InboundDatabaseHelperTests). The // shared in-memory db is seeded with Machine(Code,SAPID)=('Z28061A','131453'). private sealed class SqliteGateway : IDatabaseGateway