fix(uninstall): don't crash on malformed settings.json (#434)#469
Open
isaukywhite wants to merge 1 commit into
Open
fix(uninstall): don't crash on malformed settings.json (#434)#469isaukywhite wants to merge 1 commit into
isaukywhite wants to merge 1 commit into
Conversation
…t#434) JSON.parse on a malformed settings.json throws a SyntaxError, which has no .code, so the ENOENT-only catch re-threw it — crashing the script mid-cleanup after .ponytail-active and config.json were already deleted. Classify the error instead: ENOENT swallows, SyntaxError warns and leaves the file intact, anything else still re-throws. Adds the missing malformed-JSON test case. Co-Authored-By: Claude <noreply@anthropic.com>
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.
What
Fixes a crash in
scripts/uninstall.jswhensettings.jsoncontains malformed JSON (#434).Why
JSON.parseon a malformedsettings.jsonthrows aSyntaxError, which has no.code. The ENOENT-only catch re-threw it, crashing the script mid-cleanup — after.ponytail-activeandconfig.jsonwere already deleted (lines 21-22), leaving a non-zero exit and a danglingstatusLineentry pointing at a soon-to-be-deleted script (which then errors on the next Claude Code launch).How
Classify the error in the catch instead of checking only for ENOENT:
ENOENT→ no settings.json, nothing to clean (swallow, unchanged)SyntaxError→ malformed JSON; can't safely edit it, so warn the user and leave the file intactremoveIfExistsin the same file)SyntaxErrorcan only originate fromJSON.parsehere, so the classification is unambiguous.Test
Added the missing malformed-JSON case to
tests/uninstall.test.js: asserts exit 0, a warning is printed, and the file is left byte-for-byte intact. Closes the gap in the existing suite, which only covered valid and absentsettings.json.Note on the issue
The "settings.json may have been partially modified" concern doesn't actually occur:
JSON.parse(line 27) throws before the synchronouswriteFileSync(line 35), so the file is left untouched, never half-written. The real harm is the crash + the danglingstatusLinereference — both addressed by this fix.Closes #434