Skip to content

EPatchType.DisableTelemetry has no EnhancerConfig entry - unguarded lookup would throw KeyNotFoundException (latent) #172

Description

@divya0795

Not a live bug — no current reproduction. This is a latent crash path that today is unreachable through any supported flow. I'm filing it because it is armed by a one-line UI change, and the failure mode is disproportionate to that change. Please close if you'd rather not carry defensive code for it.

Observed on master @ 643c8f8 (1.0.9.4).

The gap

WandEnhancer/Models/PatchConfig.cs declares five patch types:

public enum EPatchType
{
    ActivatePro = 1,
    DisableUpdates = 2,
    DisableTelemetry = 4,     // ← no implementation
    DevToolsOnF12 = 8,
    RemoteWebPanelPreview = 16
}

EnhancerConfig.GetInstance() returns entries for only four of them — ActivatePro (:105), DisableUpdates (:188), DevToolsOnF12 (:204), RemoteWebPanelPreview (:225). There is no DisableTelemetry key.

Both consumers index that dictionary directly, with no containment check:

// Enhancer.cs:153, in PatchAsar
var entries = enhancerConfig[entry];

// Enhancer.cs:192, in CouldFileContainRemainingPatch
foreach (var patchEntry in enhancerConfig[patchType])

So a PatchConfig whose PatchTypes contains DisableTelemetry aborts the whole patch run with an unhandled KeyNotFoundException — killing the other requested patches too, and after Enhancer has already created backups and extracted app.asar.

Why it is currently unreachable

I checked both routes into PatchTypes before filing:

  1. UIPatchVectorsPopup builds the only PatchConfig in the codebase (PatchVectorsPopup.xaml.cs:98) and offers no DisableTelemetry control.
  2. PersistencePatchConfig is never deserialised. SettingsManager deserialises AppSettings, not PatchConfig, so a hand-edited settings file cannot inject the value either.

That is the whole of it, which is why there are no reproduction steps to give.

Why it's still worth guarding

The enum member is already public and already has a bit value allocated. Adding one checkbox to PatchVectorsPopup — the natural way to start implementing telemetry blocking — arms the crash immediately, and the resulting KeyNotFoundException gives no hint that the cause is a missing config entry.

Suggested fix

Use TryGetValue at both sites and skip a type with no definition, deliberately leaving it in remainingPatches so it surfaces through the existing summary rather than being silently dropped:

[ENHANCER] Failed to apply patches: DisableTelemetry. The version may not be supported.

That reuses the same channel a genuinely unmatched patch already reports through. A fix is attached; behaviour for all four implemented patch types is unchanged.

The alternative — adding an explicit (possibly empty) DisableTelemetry entry to EnhancerConfig — would also stop the crash, but it would make a requested patch silently succeed without doing anything, which seems worse.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions