Explain a stale latest pointer instead of leaking a raw ENOENT - #217
Merged
Conversation
`latest` resolves through a cwd-keyed pointer file, so the recording it names can be deleted after the fact (a cleaned temp dir, a trashed artifact). Every consumer verb then surfaced the filesystem's own `ENOENT ... open '/private/tmp/...'`, which names an internal path and gives the caller no next step. Resolution now checks the artifact before handing its path on: `requireArtifact` rewrites the not-there family (ENOENT/ENOTDIR) into a message naming which artifact is gone (recording, run-group manifest, CPU or allocation profile), its path through `displayPath`, and the fix. Any other errno, and every path the user typed, keeps its own error. Run-group members get the same treatment: `requireMemberRecording` is the one checked path helper every member read goes through, so the stitching verbs and the routed single-member verbs name the member rather than the errno.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defect
latestresolves through a cwd-keyed pointer file ($XDG_STATE_HOME/wpd/pointers/<hash>.json), so the recording it names can be gone by the time a verb reads it: a cleaned temp dir, a trashed artifact. Every consumer verb then printed the filesystem's own error, which names an internal path and offers no next step:Same for
assert latest,query blame latest,cpu-diff latest latest, and a run-group whose member recording was deleted.Fix
The check lives in the resolution layer, so every verb gets it:
resolve.tsgainsrequireArtifact(absPath, subject, fix). It rewrites only the not-there family (ENOENT/ENOTDIR) into a message naming which artifact is gone, its path throughdisplayPath(the same way every report shows one), and the recovery. A permission or IO failure keeps its own error, and so does every path the user typed, so a corrupt or wrong-schema file still gets the schema-epoch message.query cpu latestblames the.cpu.json, not a recording sitting right there.requireMemberRecording, used byloadMemberRecording,resolveVerbTarget, thequery spans/query spanstitches, bothassertmember reads, and the per-memberdiffpair.memberRecordingPathstays as the pure path and is no longer exported.assertMemberModealso shows its path throughdisplayPath.After:
Exit status is unchanged (non-zero).
Tests
test/unit/stale-pointer.test.mjs(browser-free, against compileddist/,XDG_STATE_HOMEpointed at a temp dir):resolveTargetandresolveConsumptionENOTDIR(a path under a file) worded as the same not-there caseloadMemberRecordingandresolveVerbTargetTwo existing tests pointed
latestat files that were never created; they now create them, rather than weakening the new check.Gates: build, 761 unit tests, lint, format:check, knip all pass. Verified on the real CLI for both the plain and the run-group case.