Skip to content

One truncated or unparseable snapshot file makes SnapshotStore.History throw for that object permanently, and Add never repairs it #62

Closed

Description

@matt-edmondson

What's wrong

SnapshotStore is the append-only archive that the M2 divergence measurement depends on. Two of its behaviours combine so that one bad file takes out an object's entire history, permanently.

  1. Add writes each record in place with File.WriteAllText, and it skips any path that already exists:

    /// <exception cref="ArgumentNullException"><paramref name="ommJson"/> is null.</exception>
    public int Add(string ommJson)
    {
    Ensure.NotNull(ommJson);
    int added = 0;
    foreach ((ElementSet elements, string record) in OmmJson.ReadWithSource(ommJson))
    {
    string path = PathFor(elements.NoradCatalogId, elements.Epoch);
    if (File.Exists(path))
    {
    continue;
    }
    System.IO.Directory.CreateDirectory(Path.GetDirectoryName(path)!);
    File.WriteAllText(path, record);
    added++;
    }
    return added;

    If the process dies partway through a write, a truncated <epoch>.json is left under its real name. Every later Add of the same element set sees File.Exists(path) and continues, so the file is never rewritten.

  2. History parses every file in the object's folder with OmmJson.Read, with no error handling:

    public IReadOnlyList<ElementSet> History(int noradCatalogId)
    {
    string folder = FolderFor(noradCatalogId);
    if (!System.IO.Directory.Exists(folder))
    {
    return [];
    }
    List<ElementSet> history = [];
    foreach (string path in System.IO.Directory.EnumerateFiles(folder, "*.json"))
    {
    history.AddRange(OmmJson.Read(File.ReadAllText(path)));
    }
    history.Sort((left, right) => left.Epoch.CompareTo(right.Epoch));
    return history;
    }

    One unreadable file throws, and the caller loses every good element set for that object.

There is a second way into the same state that needs no crash. The class doc says the store keeps the source's own JSON so that "if the parser changes, or turns out to have been wrong, what the source actually served is still here to re-read". Once the parser is made stricter (for example #58, which rejects a missing MEAN_MOTION / NORAD_CAT_ID, or #54, EPOCH formats), any archived record the new parser rejects makes History throw for the whole object. The archive becomes unreadable exactly when it was meant to help.

This is separate from #59. That issue covers ResponseCache.Write, where the next successful fetch overwrites the bad file. Nothing ever overwrites a snapshot file.

Failure scenario (reproduced against 0a3ba17)

The run used the ISS OMM fixture from CelesTrakClientTests:

first add: 1
re-add after truncation: 0 (0 = skipped, file not repaired)
History threw JsonReaderException: Expected depth to be zero at the end of the JSON payload. ...

Add succeeds, and the archived file is then cut to half its length to simulate an interrupted write. Add of the same data returns 0 and History(25544) throws. The ISS history stays unreadable until someone deletes the file by hand, and nothing tells them which file it is.

Suggested fix

  • Write each snapshot atomically: write to a temp file beside it, then File.Move(tmp, path). History already filters on *.json, so stray temp files are ignored.
  • In Add, when the path exists but does not parse, replace it with the incoming record instead of skipping it.
  • In History, skip and report a file that fails to parse (for example, return or log the unreadable paths) instead of discarding the whole object's history.

Acceptance criteria

  • A test truncates an archived file. History still returns the other element sets for that object, and a later Add of the same record restores the file.
  • A test places a record the current parser rejects in the folder. History does not throw and still returns the readable sets.
  • No code path leaves a partial file under a real <epoch>.json name.

Activity

  1. matt-edmondson commented on Oct 5, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions