From 3c8837e21ee3e7ace419cfdd04fcef0c50d42140 Mon Sep 17 00:00:00 2001
From: ImanThiyanga <ithiyanga@gmail.com>
Date: Wed, 7 Oct 2026 11:53:31 +0530
Subject: [PATCH] Stop the GL account-code check from being able to abort
 startup

The catch filter only matched DomainException/OperationCanceledException,
so anything else escaped a hosted service and took the host down. One such
path was mine: `"items": null` deserializes over the property initialiser,
so `data.Items.Select(...)` would NullReferenceException straight past the
filter.

Fixed at both ends - the reader tolerates a null or absent items array, and
the catch is now a deliberate catch-all that only lets a genuine host
shutdown through. This is a diagnostic; a diagnostic that can core-dump the
thing it diagnoses is worse than no diagnostic at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../GlAccountCodeValidatorTests.cs            | 24 +++++++++++++++++++
 .../Infra/Gl/GlAccountCodeValidator.cs        |  9 +++----
 .../ERPCore/Services/GeneralLedgerService.cs  |  7 +++---
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Backend/ERPCore.Tests/GlAccountCodeValidatorTests.cs b/Backend/ERPCore.Tests/GlAccountCodeValidatorTests.cs
index 73196d9..4e88af7 100644
--- a/Backend/ERPCore.Tests/GlAccountCodeValidatorTests.cs
+++ b/Backend/ERPCore.Tests/GlAccountCodeValidatorTests.cs
@@ -91,6 +91,30 @@ public class GlAccountCodeValidatorTests
         await validator.StartAsync(CancellationToken.None);
     }
 
+    /// <summary>
+    /// The earlier catch filter only matched DomainException/OperationCanceledException, so anything
+    /// else — a parse failure, or a null `items` NRE in the reader — escaped and aborted the host.
+    /// A diagnostic must not be able to crash the thing it is diagnosing.
+    /// </summary>
+    [Theory]
+    [MemberData(nameof(UnexpectedFailures))]
+    public async Task StartAsync_DoesNotThrow_OnAnyUnexpectedFailure(Exception failure)
+    {
+        var validator = CreateValidator(
+            new() { ["Grn:GlInventoryAccountCode"] = "1100" },
+            new StubGl(failure));
+
+        await validator.StartAsync(CancellationToken.None);
+    }
+
+    public static TheoryData<Exception> UnexpectedFailures() =>
+    [
+        new NullReferenceException("items was null"),
+        new global::System.Text.Json.JsonException("unexpected shape"),
+        new InvalidOperationException("something else entirely"),
+        new HttpRequestException("connection reset"),
+    ];
+
     [Fact]
     public async Task StartAsync_DoesNotThrow_WhenTheChartOfAccountsReadIsCancelled()
     {
diff --git a/Backend/ERPCore/Infra/Gl/GlAccountCodeValidator.cs b/Backend/ERPCore/Infra/Gl/GlAccountCodeValidator.cs
index ece1c0a..61b4ff4 100644
--- a/Backend/ERPCore/Infra/Gl/GlAccountCodeValidator.cs
+++ b/Backend/ERPCore/Infra/Gl/GlAccountCodeValidator.cs
@@ -1,5 +1,4 @@
 using ERPCore.Services.Interfaces;
-using ERPCore.System.Errors;
 
 namespace ERPCore.Infra.Gl;
 
@@ -61,10 +60,12 @@ public sealed class GlAccountCodeValidator(
             timeout.CancelAfter(FetchTimeout);
             present = await gl.ListAccountCodesAsync(timeout.Token);
         }
-        catch (Exception e) when (e is DomainException or OperationCanceledException && !ct.IsCancellationRequested)
+        // Catch-all on purpose. This is a diagnostic, and a diagnostic that can abort the host is
+        // worse than no diagnostic — whatever GL answers with (unreachable, a timeout, a redirect,
+        // an unparseable body, a shape this reader mishandles) must degrade to a log line, never a
+        // core-dump. The only exception that still propagates is a genuine host shutdown.
+        catch (Exception e) when (!ct.IsCancellationRequested)
         {
-            // Unreachable, timed out, or rejected the read — can't verify, so don't block startup.
-            // The per-flow 404 remains the backstop it has always been.
             logger.LogWarning(e,
                 "Could not read GL's chart of accounts at startup; {Count} configured account code(s) left unverified.",
                 expected.Length);
diff --git a/Backend/ERPCore/Services/GeneralLedgerService.cs b/Backend/ERPCore/Services/GeneralLedgerService.cs
index a177270..4969b36 100644
--- a/Backend/ERPCore/Services/GeneralLedgerService.cs
+++ b/Backend/ERPCore/Services/GeneralLedgerService.cs
@@ -56,9 +56,10 @@ public sealed class GeneralLedgerService : IGeneralLedgerService
         // `parentAccountId` is a field, not a nested `children` array).
         var response = await _client.SendAsync(HttpMethod.Get, "accounts", null, null, null, ct);
         var data = ParseSuccess<GlAccountListData>(response);
-        return data.Items
+        // `"items": null` deserializes over the property initialiser, so the null check is load-bearing.
+        return (data.Items ?? [])
+            .Where(a => a is not null && !string.IsNullOrWhiteSpace(a.AccountCode))
             .Select(a => a.AccountCode)
-            .Where(c => !string.IsNullOrWhiteSpace(c))
             .ToHashSet(StringComparer.Ordinal);
     }
 
@@ -111,7 +112,7 @@ public sealed class GeneralLedgerService : IGeneralLedgerService
 
     private sealed class GlAccountListData
     {
-        public List<GlAccountData> Items { get; set; } = [];
+        public List<GlAccountData>? Items { get; set; }
     }
 
     private sealed class GlAccountData