Skip to content

fix: skip corrupted run state files in list_runs - #3817

Open
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/json-decode-error-in-runstate
Open

fix: skip corrupted run state files in list_runs#3817
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/json-decode-error-in-runstate

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Wrap json.load() in workflows/engine.py list_runs with ry/except (json.JSONDecodeError, OSError) so a single corrupted state.json doesn't crash the entire listing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Improves workflow run listing resilience by skipping unreadable or malformed state files.

Changes:

  • Handles JSON parsing and file-access failures in list_runs().
Show a summary per file
File Description
src/specify_cli/workflows/engine.py Skips state files that raise JSON or I/O errors.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread src/specify_cli/workflows/engine.py
Comment thread src/specify_cli/workflows/engine.py
…n, and regression tests

- Catch UnicodeDecodeError for invalid UTF-8 encoding
- Validate loaded JSON is a dict with required 'run_id' key
- Add 5 regression tests for corrupted state files

Fixes github#3817

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (1)

tests/test_workflows.py:6224

  • This does not reliably create an unreadable file: Windows' attrib +R only prevents writes (and the branch explicitly expects the read to succeed), while POSIX mode bits are bypassed by privileged/root test processes. As a result, the new OSError handling is untested on Windows and this test can fail in root-based containers. Please monkeypatch the file-opening call to raise PermissionError for this state file so the exception path is deterministic on every platform.
        if sys.platform == "win32":
            subprocess.run(["attrib", "+R", str(state_file)], check=True)
        else:
            state_file.chmod(0o000)
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants