Four hardening items examined during #90's review, judged real but not worth fixing there. Each needs an input or timing that normal use never produces, so they are recorded here with the analysis done, in case one ever surfaces in the wild.
1. A serialization failure during apply escapes the rollback contract
FanatecLedModuleHost.Apply snapshots the module (JToken.FromObject) for rollback before entering its own try block. If that serialization ever throws — it walks SimHub's module object, whose property getters are not ours — the exception bypasses "return false, restore state" and propagates; before publication that orphans the device until restart. Separately, a hand-edited "ledModuleSettings": null passes the != null reference check and fails inside the guard, faulting the device instead of being treated as "nothing to apply".
If acting: move the snapshot inside the try (it is the first operation, so a snapshot failure means an untouched module and returning false is correct), and guard with is JObject. Note the real host cannot run under unit test, so this is verifiable only by reading.
2. A wrong-typed known field orphans the device
FanatecSettingsSnapshot.FromJson tolerates absent and null values but not incompatible ones: (byte?) on 300 overflows, (bool?) on "yes" fails to parse. A document from a build that changed a field's type would throw during load — orphaning the device where falling back to that one field's default would do.
If acting: tolerant per-field readers that default on any conversion failure, plus a fixture with hostile values.
3. Policy: should rejected settings orphan an unpublished device?
A payload the LED module rejects during load disposes the host and rethrows, so SimHub parks the device as an orphan — settings file preserved, device invisible until restart. That is the deliberate fail-closed policy: never publish a device holding a mixture nobody chose. The alternative is publishing it faulted (visible, not saving, not driving LEDs), which is friendlier and self-explanatory but weakens the invariant.
If acting: this is a decision, not a bug fix. Revisit only if a user ever reports a vanished device alongside a rejected-settings log line.
4. The registration-time settings reader is stricter and deeper than SimHub's own
PersistedPluginSettings exists so device registration can resolve profile overrides without the plugin running. It differs from how SimHub itself loads the same file in two ways:
- SimHub deserializes the whole file into the typed settings class; ours accepts any file that parses as JSON. A file that parses but would fail typed deserialization reads differently for registration than for the running plugin.
- SimHub writes ten rolling backups but consults only five when reading; ours walks all ten, so a value in
_b6–_b10 could be honoured at registration that SimHub itself would never load.
Either divergence takes a type-mismatched document or more than five consecutive unreadable files to matter.
If acting: align both behaviours (typed read, five backups) and replace the hand-written candidate-walk tests with a contract test. Do not introduce a PluginManager.Instance dependency at registration time — independence from the singleton is the reason this reader exists.
Four hardening items examined during #90's review, judged real but not worth fixing there. Each needs an input or timing that normal use never produces, so they are recorded here with the analysis done, in case one ever surfaces in the wild.
1. A serialization failure during apply escapes the rollback contract
FanatecLedModuleHost.Applysnapshots the module (JToken.FromObject) for rollback before entering its own try block. If that serialization ever throws — it walks SimHub's module object, whose property getters are not ours — the exception bypasses "return false, restore state" and propagates; before publication that orphans the device until restart. Separately, a hand-edited"ledModuleSettings": nullpasses the!= nullreference check and fails inside the guard, faulting the device instead of being treated as "nothing to apply".If acting: move the snapshot inside the try (it is the first operation, so a snapshot failure means an untouched module and returning false is correct), and guard with
is JObject. Note the real host cannot run under unit test, so this is verifiable only by reading.2. A wrong-typed known field orphans the device
FanatecSettingsSnapshot.FromJsontolerates absent and null values but not incompatible ones:(byte?)on300overflows,(bool?)on"yes"fails to parse. A document from a build that changed a field's type would throw during load — orphaning the device where falling back to that one field's default would do.If acting: tolerant per-field readers that default on any conversion failure, plus a fixture with hostile values.
3. Policy: should rejected settings orphan an unpublished device?
A payload the LED module rejects during load disposes the host and rethrows, so SimHub parks the device as an orphan — settings file preserved, device invisible until restart. That is the deliberate fail-closed policy: never publish a device holding a mixture nobody chose. The alternative is publishing it faulted (visible, not saving, not driving LEDs), which is friendlier and self-explanatory but weakens the invariant.
If acting: this is a decision, not a bug fix. Revisit only if a user ever reports a vanished device alongside a rejected-settings log line.
4. The registration-time settings reader is stricter and deeper than SimHub's own
PersistedPluginSettingsexists so device registration can resolve profile overrides without the plugin running. It differs from how SimHub itself loads the same file in two ways:_b6–_b10could be honoured at registration that SimHub itself would never load.Either divergence takes a type-mismatched document or more than five consecutive unreadable files to matter.
If acting: align both behaviours (typed read, five backups) and replace the hand-written candidate-walk tests with a contract test. Do not introduce a
PluginManager.Instancedependency at registration time — independence from the singleton is the reason this reader exists.