Repository navigation
One truncated or unparseable snapshot file makes SnapshotStore.History throw for that object permanently, and Add never repairs it #62
Description
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.
-
Addwrites each record in place withFile.WriteAllText, and it skips any path that already exists:
Osculator/Osculator.Data/CelesTrak/SnapshotStore.cs
Lines 46 to 67 in 0a3ba17
/// <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>.jsonis left under its real name. Every laterAddof the same element set seesFile.Exists(path)andcontinues, so the file is never rewritten. -
Historyparses every file in the object's folder withOmmJson.Read, with no error handling:
Osculator/Osculator.Data/CelesTrak/SnapshotStore.cs
Lines 75 to 94 in 0a3ba17
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).Historyalready 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.
Historystill returns the other element sets for that object, and a laterAddof the same record restores the file. - A test places a record the current parser rejects in the folder.
Historydoes not throw and still returns the readable sets. - No code path leaves a partial file under a real
<epoch>.jsonname.
Activity
matt-edmondson commentedon Oct 5, 2026
Triage
Osculator.Data/CelesTrak/SnapshotStore(Add,History)ResponseCache.Writeatomicity), which can use the same temp-then-move helper. Land this before or together with OmmJson turns a missing MEAN_MOTION, ECCENTRICITY or NORAD_CAT_ID into 0 instead of rejecting the record, so an incomplete document passes CelesTrak's parse-before-cache check #58 so the stricter parser doesn't make existing archives unreadable.File.Move), replace an existing file that does not parse inAdd, and skip and report unparseable files inHistory.Generated by Claude Code