Skip to content

fix: treat empty on-disk JSON state files as empty state#616

Open
mwbrooks wants to merge 3 commits into
mainfrom
mwbrooks-fix-unable-to-parse-json
Open

fix: treat empty on-disk JSON state files as empty state#616
mwbrooks wants to merge 3 commits into
mainfrom
mwbrooks-fix-unable-to-parse-json

Conversation

@mwbrooks

@mwbrooks mwbrooks commented Jul 15, 2026

Copy link
Copy Markdown
Member

Changelog

Fixed a recurring unable_to_parse_json error caused by empty JSON files (credentials, config, or app state) left behind by an interrupted write - the CLI now treats them as empty state and recovers automatically.

Summary

This pull request helps to address unable_to_parse_json internal errors that some users are experiencing.

Problem

afero.WriteFile opens with O_TRUNC before writing. If the process is interrupted (SIGKILL, host crash, oom, user cancel) after the truncate but before the write completes, the JSON state file is left at zero bytes. Every subsequent CLI invocation then reads the file, calls json.Unmarshal(nil, …) on the empty contents, and raises ErrUnableToParseJSON. The file never self-heals: the reader errors out before the caller reaches the code path that would rewrite it.

Affected state files:

  • ~/.slack/credentials.json
  • ~/.slack/config.json
  • Project .slack/config.json
  • Project .slack/apps.json
  • Project .slack/apps.dev.json

What this PR changes

Adds a single helper goutils.IsEmptyJSON([]byte) bool and updates the five readers so that an empty (or whitespace-only) file returns the same zero-value state as the "file does not exist" branch, rather than surfacing a parse error:

  • internal/auth/auth.goauths() returns an empty AuthByTeamID
  • internal/config/system.goUserConfig returns a config with Surveys initialised
  • internal/config/project.goReadProjectConfigFile returns a config with Surveys initialised
  • internal/app/app_client.goreadDeployedApps and readLocalApps return empty maps and re-serialise a valid {} so the file self-heals on the next write cycle

Genuinely malformed JSON still returns ErrUnableToParseJSON. Existing broken-JSON tests are untouched.

Test plan

  • Test_IsEmptyJSON covers empty, whitespace-only, {}, and populated inputs
  • Test_Client_auths empty-file subtest returns no auths, no error
  • Test_SystemConfig_UserConfig empty-file subtest returns zero-valued config, no error
  • Test_ReadProjectConfigFile empty-file subtest returns zero-valued config, no error
  • AppClient.readDeployedApps empty-file subtest returns empty apps and writes {} back to disk
  • AppClient.readLocalApps empty-file subtest returns empty local apps and writes {} back to disk
  • go test ./internal/... ./cmd/... passes
  • make lint passes with zero issues

Follow-ups (not in this PR)

  • Atomic writes — replace afero.WriteFile with a temp-file + Rename pattern across the five state writers so the durable fix is that these files can never reach a 0-byte state in the first place.
  • Centralise the read-JSON-state pattern — the "stat → read → check empty → unmarshal → wrap error" sequence is now duplicated across five call sites. Move it into goutils (or a new internal/statefile package) so future state files pick up the guard for free.
  • internal/api/activity.goErrHTTPResponseInvalid on the activity poll loop shows the same "thousands of events off one broken response" shape; worth a matching guard.
  • internal/update/sdk.go — retry with backoff on persistently malformed SDK hook response so a broken SDK doesn't dominate the error telemetry either.

… error

Truncated writes to the CLI's JSON state files (credentials.json,
~/.slack/config.json, project .slack/config.json, .slack/apps.json,
.slack/apps.dev.json) can leave a file at zero bytes on disk when a prior
process is interrupted between afero.WriteFile's O_TRUNC and the actual
write. Once that happens, every subsequent CLI invocation reads the same
0-byte file, hits "unexpected end of JSON input" in the read helper, and
raises ErrUnableToParseJSON.

The June 2026 monthly report showed 1,317,610 unable_to_parse_json events
from just 38 users (a hot loop concentrated in a small cohort running
`platform run` and `api` in tight automation loops), which matches this
failure mode: a file that never repairs itself keeps re-triggering on
every invocation.

Add a shared goutils.IsEmptyJSON helper and use it in the five hot
readers (auth credentials, system config, project config, deployed apps,
local dev apps). When the file exists but is empty or whitespace-only,
return the same zero-value state the "file does not exist" branch
already returns — the app_client readers also re-serialise a valid `{}`
so the file self-heals on the next successful write. Genuinely malformed
JSON still returns ErrUnableToParseJSON as before.

Test coverage: added unit tests covering zero-byte and whitespace-only
inputs for each of the five readers, plus a unit test for the
IsEmptyJSON helper itself. Existing "broken JSON" tests are unchanged.
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.74%. Comparing base (a265b95) to head (77804be).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
internal/app/app_client.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   71.72%   71.74%   +0.02%     
==========================================
  Files         226      226              
  Lines       19182    19204      +22     
==========================================
+ Hits        13758    13778      +20     
  Misses       4214     4214              
- Partials     1210     1212       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mwbrooks mwbrooks self-assigned this Jul 16, 2026
@mwbrooks mwbrooks added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment labels Jul 16, 2026

@mwbrooks mwbrooks left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments for the kind reviewers

Comment thread internal/config/system.go
// every subsequent CLI invocation reads the same 0-byte file and logs
// ErrUnableToParseJSON in a hot loop.
if goutils.IsEmptyJSON(configFileBytes) {
config.Surveys = map[string]SurveyConfig{}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I thought there might be an initialize function, but this same pattern is used elsewhere.

@mwbrooks
mwbrooks marked this pull request as ready for review July 16, 2026 23:10
@mwbrooks
mwbrooks requested a review from a team as a code owner July 16, 2026 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant