60ce1c9d57
Phase 15 complete (2 plans):
- 15-01: NLog 5.2.8 + nlog.config (rolling file daily/7d + console); logger fields in
FanucMachine, HeidenhainMachine, SiemensMachine, MitsubishiMachine, Lsv2Client, Program.cs;
SetState event handler catches → _log.Warn; cleanup catches annotated; ISS-003/ISS-004 closed
- 15-02: All Console.Write/WriteLine in Program.cs replaced with _log.Info (103 calls);
nlog.config console target layout=${message}, levels=Info only — Warn/Error file-only;
operator console output identical; every interaction now timestamped in log file
Milestone v0.5 Operational Quality complete.
Co-Authored-By: Claude <noreply@anthropic.com>
317 lines
12 KiB
Markdown
317 lines
12 KiB
Markdown
---
|
||
phase: 15-logging
|
||
plan: 01
|
||
type: execute
|
||
wave: 1
|
||
depends_on: []
|
||
files_modified:
|
||
- packages.config
|
||
- NcProgramManager.csproj
|
||
- nlog.config
|
||
- Program.cs
|
||
- Cnc/Fanuc/FanucMachine.cs
|
||
- Cnc/Heidenhain/HeidenhainMachine.cs
|
||
- Cnc/Heidenhain/Lsv2Client.cs
|
||
- Cnc/Siemens/SiemensMachine.cs
|
||
- Cnc/Mitsubishi/MitsubishiMachine.cs
|
||
autonomous: true
|
||
---
|
||
|
||
<objective>
|
||
## Goal
|
||
Add NLog file+console logging and surface silent exception swallows in event handler paths across all 4 machine classes.
|
||
|
||
## Purpose
|
||
ISS-003: 18 silent `catch {}` blocks mean production failures are undiagnosable. ISS-004: all output goes to console only — operators cannot send a log file for support. Adding NLog with a rolling file target gives a persistent record for debugging.
|
||
|
||
## Output
|
||
- `nlog.config` — rolling file log + console, next to EXE
|
||
- NLog wired into 5 production classes via `LogManager.GetCurrentClassLogger()`
|
||
- Event handler catch blocks changed from silent swallow to `_log.Warn`
|
||
- Dispose/cleanup catch blocks annotated as intentionally silent (no logging — Dispose must not rethrow)
|
||
- Program.cs error catch blocks emit `_log.Error` in addition to existing `Console.WriteLine`
|
||
</objective>
|
||
|
||
<context>
|
||
@.paul/PROJECT.md
|
||
@.paul/ISSUES.md
|
||
@Cnc/Siemens/SiemensMachine.cs
|
||
@Cnc/Heidenhain/Lsv2Client.cs
|
||
</context>
|
||
|
||
<acceptance_criteria>
|
||
|
||
## AC-1: NLog config present and wired
|
||
```gherkin
|
||
Given nlog.config is placed next to NcProgramManager.exe
|
||
When the application starts
|
||
Then NLog initialises without throwing (no "NLog configuration file not found" in console)
|
||
And a logs/ subdirectory is created containing nc_program_manager.log
|
||
```
|
||
|
||
## AC-2: Event handler exceptions reach the log
|
||
```gherkin
|
||
Given a StateChanged handler that throws
|
||
When SetState() fires in any machine class
|
||
Then the exception is caught and logged at WARN level (not silently swallowed)
|
||
```
|
||
|
||
## AC-3: Dispose cleanup catch blocks are annotated
|
||
```gherkin
|
||
Given all Dispose() and disconnect cleanup catch blocks
|
||
When reviewed in code
|
||
Then each has a // cleanup — intentionally swallowed comment (no log call — Dispose must not rethrow)
|
||
```
|
||
|
||
## AC-4: Program.cs errors reach the log file
|
||
```gherkin
|
||
Given any unhandled exception in Scarica, Invia, or arg parsing
|
||
When the catch block runs
|
||
Then _log.Error(ex, ...) is called BEFORE Console.WriteLine
|
||
And all existing Console.WriteLine calls remain unchanged
|
||
```
|
||
|
||
</acceptance_criteria>
|
||
|
||
<tasks>
|
||
|
||
<task type="auto">
|
||
<name>Task 1: Add NLog package and nlog.config</name>
|
||
<files>
|
||
packages.config,
|
||
NcProgramManager.csproj,
|
||
nlog.config
|
||
</files>
|
||
<action>
|
||
1. In `packages.config`, add inside `<packages>`:
|
||
```xml
|
||
<package id="NLog" version="5.2.8" targetFramework="net472" />
|
||
```
|
||
|
||
2. In `NcProgramManager.csproj`, inside the existing `<ItemGroup>` that contains `<Reference>` entries, add:
|
||
```xml
|
||
<Reference Include="NLog, Version=5.0.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c, processorArchitecture=MSIL">
|
||
<HintPath>packages\NLog.5.2.8\lib\net46\NLog.dll</HintPath>
|
||
<Private>True</Private>
|
||
</Reference>
|
||
```
|
||
|
||
Also in `NcProgramManager.csproj`, inside the `<ItemGroup>` that contains `<Compile>` entries (or a new `<ItemGroup>`), add:
|
||
```xml
|
||
<Content Include="nlog.config">
|
||
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
|
||
</Content>
|
||
```
|
||
|
||
3. Create `nlog.config` in the project root:
|
||
```xml
|
||
<?xml version="1.0" encoding="utf-8" ?>
|
||
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd"
|
||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||
autoReload="true"
|
||
throwConfigExceptions="true">
|
||
|
||
<targets>
|
||
<target xsi:type="File"
|
||
name="logfile"
|
||
fileName="${basedir}/logs/nc_program_manager.log"
|
||
archiveFileName="${basedir}/logs/nc_program_manager.{#}.log"
|
||
archiveEvery="Day"
|
||
archiveNumbering="Date"
|
||
maxArchiveFiles="7"
|
||
layout="${longdate} ${level:uppercase=true:padding=5} [${logger:shortName=true}] ${message}${onexception:${newline}${exception:format=tostring}}" />
|
||
|
||
<target xsi:type="Console"
|
||
name="console"
|
||
layout="${level:uppercase=true:padding=5} [${logger:shortName=true}] ${message}" />
|
||
</targets>
|
||
|
||
<rules>
|
||
<logger name="*" minlevel="Debug" writeTo="logfile" />
|
||
<logger name="*" minlevel="Warn" writeTo="console" />
|
||
</rules>
|
||
|
||
</nlog>
|
||
```
|
||
|
||
Constraints:
|
||
- NLog package version is 5.2.8 — supports .NET 4.6+ (project is 4.7.2)
|
||
- lib/net46 is the highest .NET Framework target in the NLog 5.x package
|
||
- Console rule minlevel=Warn: logger output must NOT duplicate user-facing Console.WriteLine in Program.cs
|
||
- `throwConfigExceptions="true"` surfaces config errors at startup rather than silently failing
|
||
</action>
|
||
<verify>
|
||
- Read packages.config: NLog 5.2.8 entry present
|
||
- Read NcProgramManager.csproj: Reference to NLog with HintPath present; nlog.config Content entry with CopyToOutputDirectory present
|
||
- Read nlog.config: file target with archiving, console target, two rules present
|
||
</verify>
|
||
<done>AC-1 satisfied: NLog wired into build; nlog.config copied to output directory on build</done>
|
||
</task>
|
||
|
||
<task type="auto">
|
||
<name>Task 2: Wire logger into machine classes and fix event handler catches</name>
|
||
<files>
|
||
Cnc/Fanuc/FanucMachine.cs,
|
||
Cnc/Heidenhain/HeidenhainMachine.cs,
|
||
Cnc/Heidenhain/Lsv2Client.cs,
|
||
Cnc/Siemens/SiemensMachine.cs,
|
||
Cnc/Mitsubishi/MitsubishiMachine.cs
|
||
</files>
|
||
<action>
|
||
For each of the 5 files:
|
||
|
||
**A. Add using directive at top:**
|
||
```csharp
|
||
using NLog;
|
||
```
|
||
|
||
**B. Add logger field** inside the class, after existing private fields:
|
||
```csharp
|
||
private static readonly Logger _log = LogManager.GetCurrentClassLogger();
|
||
```
|
||
|
||
**C. Fix event handler catch blocks** — there is one `SetState` method in each machine class containing:
|
||
```csharp
|
||
try { h(this, next); } catch { }
|
||
```
|
||
Replace with:
|
||
```csharp
|
||
try { h(this, next); } catch (Exception ex) { _log.Warn(ex, "StateChanged handler threw"); }
|
||
```
|
||
|
||
**D. Annotate Dispose() cleanup catches** — do NOT add logging (Dispose must not rethrow):
|
||
```csharp
|
||
try { DisconnectAsync().GetAwaiter().GetResult(); } catch { /* cleanup — intentionally swallowed */ }
|
||
try { _gate.Dispose(); } catch { /* cleanup — intentionally swallowed */ }
|
||
```
|
||
|
||
**For Lsv2Client specifically (no SetState, no _gate):**
|
||
- Add `using NLog;` and `private static readonly Logger _log = LogManager.GetCurrentClassLogger();`
|
||
- In `Connect()` timeout branch: `try { _tcp.Close(); } catch { /* cleanup — intentionally swallowed */ }`
|
||
- In `Disconnect()`: annotate all 3 cleanup catches with `/* cleanup — intentionally swallowed */`
|
||
- Add connect/disconnect info logging:
|
||
```csharp
|
||
// At start of Connect(), before _tcp = new TcpClient():
|
||
_log.Info("Connecting to {0}:{1}", _config.IpAddress, _config.Port);
|
||
// After successful Login():
|
||
_log.Info("Connected to {0}", _config.IpAddress);
|
||
// At start of Disconnect():
|
||
_log.Info("Disconnecting from {0}", _config.IpAddress);
|
||
```
|
||
|
||
Constraints:
|
||
- Do NOT add logging inside `Dispose()` or disposal catch blocks — Dispose must not throw and logging adds failure risk
|
||
- Do NOT change `catch (Exception ex) { _errors.Add(...); }` blocks — already handled correctly
|
||
- Do NOT change `catch (OperationCanceledException)` blocks — correct behavior
|
||
- Logger is `static readonly` (one per class type, not per instance) — NLog standard pattern
|
||
- Do NOT add `using NLog;` if already present (check first)
|
||
</action>
|
||
<verify>
|
||
- Grep for `LogManager.GetCurrentClassLogger()` in all 5 files — must appear exactly once per file
|
||
- Grep for `try { h(this, next); } catch { }` — must return 0 results (all replaced)
|
||
- Grep for `_log.Warn(ex` — must appear in FanucMachine, HeidenhainMachine, SiemensMachine, MitsubishiMachine
|
||
- Read Lsv2Client.cs: verify info logging at connect/disconnect, cleanup catch annotations present
|
||
</verify>
|
||
<done>AC-2 and AC-3 satisfied: event handler exceptions logged at WARN; cleanup catches annotated as intentional</done>
|
||
</task>
|
||
|
||
<task type="auto">
|
||
<name>Task 3: Add file logging to Program.cs error catch blocks</name>
|
||
<files>
|
||
Program.cs
|
||
</files>
|
||
<action>
|
||
1. Add `using NLog;` to the using directives at top of Program.cs.
|
||
|
||
2. Add logger field inside the `Program` class (or at the start of Main — static field):
|
||
```csharp
|
||
private static readonly Logger _log = LogManager.GetCurrentClassLogger();
|
||
```
|
||
|
||
3. In the arg parsing catch block (currently around line 19):
|
||
```csharp
|
||
catch (Exception e)
|
||
{
|
||
_log.Error(e, "Arg parsing failed");
|
||
Console.Write("Gestore Programmi FANUC: ");
|
||
Console.WriteLine(e.Message);
|
||
// ... rest unchanged
|
||
```
|
||
|
||
4. In the `Scarica` catch block (currently around line 157):
|
||
```csharp
|
||
catch (Exception ex)
|
||
{
|
||
_log.Error(ex, "Scarica failed");
|
||
Console.WriteLine(ex.Message);
|
||
// ... rest unchanged
|
||
```
|
||
|
||
5. In the `Invia` catch block (currently around line 253):
|
||
```csharp
|
||
catch (Exception ex)
|
||
{
|
||
_log.Error(ex, "Invia failed");
|
||
Console.WriteLine(ex.Message);
|
||
// ... rest unchanged
|
||
```
|
||
|
||
Constraints:
|
||
- `_log.Error(ex, ...)` MUST come BEFORE `Console.WriteLine(ex.Message)` — log first, then display
|
||
- ALL existing Console.WriteLine calls remain exactly as-is — do not modify or remove any
|
||
- Do NOT add logging for license failure (`Console.WriteLine("Licenza ERRATA")`) — that's a known condition, not an exception
|
||
- Do NOT add a `NLog.LogManager.Shutdown()` call — not needed for a short-lived console EXE
|
||
</action>
|
||
<verify>
|
||
- Read Program.cs: `using NLog;` present
|
||
- Read Program.cs: `_log` field present
|
||
- Grep for `_log.Error` in Program.cs — must appear 3 times
|
||
- Grep for `Console.WriteLine` in Program.cs — must still be 97 (unchanged count)
|
||
</verify>
|
||
<done>AC-4 satisfied: all 3 Program.cs error paths log to file before Console.WriteLine; no Console.WriteLine removed</done>
|
||
</task>
|
||
|
||
</tasks>
|
||
|
||
<boundaries>
|
||
|
||
## DO NOT CHANGE
|
||
- `Licensing/LicenseValidator.cs` — public key constant must not be modified
|
||
- `Licensing/MachineFingerprint.cs` — fixed in Phase 13; not in scope
|
||
- `NcProgramManager.Tests/` — no test changes in this phase
|
||
- `Cnc/**/` catch blocks already handling exceptions via `_errors.Add(...)` — correct, leave alone
|
||
- All existing `Console.WriteLine` calls — supplemented only, never replaced
|
||
|
||
## SCOPE LIMITS
|
||
- No logging inside Dispose() or cleanup catch paths — comment only
|
||
- No replacement of user-facing Console.WriteLine — file logging supplements via NLog
|
||
- No logging for known control-flow conditions (license failure, validation failure) — only unexpected exceptions
|
||
- No async/await changes — ISS-002 is a separate phase
|
||
- No NuGet package other than NLog — no wrapper abstraction, no DI container
|
||
|
||
</boundaries>
|
||
|
||
<verification>
|
||
Before declaring plan complete:
|
||
- [ ] `packages.config` contains NLog 5.2.8 entry
|
||
- [ ] `NcProgramManager.csproj` has NLog Reference with HintPath and nlog.config as Content/CopyAlways
|
||
- [ ] `nlog.config` exists with file target (archiving) + console target (Warn+) + rules
|
||
- [ ] All 5 machine/client files: `LogManager.GetCurrentClassLogger()` field present
|
||
- [ ] `try { h(this, next); } catch { }` — zero occurrences remain in codebase
|
||
- [ ] Dispose cleanup catches in all 4 machine classes: annotated with comment
|
||
- [ ] Lsv2Client.Disconnect() catches: annotated with comment
|
||
- [ ] Program.cs: 3× `_log.Error` added before Console.WriteLine; 97× Console.WriteLine count unchanged
|
||
</verification>
|
||
|
||
<success_criteria>
|
||
- All 3 tasks completed
|
||
- All 4 ACs covered
|
||
- ISS-003 addressed: event handler swallows surfaced; cleanup swallows documented
|
||
- ISS-004 addressed: NLog file+console logging wired into all production classes
|
||
- No existing tests broken (no test files modified)
|
||
- No production behavior changed for operators (Console.WriteLine output identical)
|
||
</success_criteria>
|
||
|
||
<output>
|
||
After completion, create `.paul/phases/15-logging/15-01-SUMMARY.md`
|
||
</output>
|