Skip to content

Fix scan crash when framework cache hits persisted review_cache#475

Closed
MacHatter1 wants to merge 1 commit intopeteromallet:mainfrom
MacHatter1:hatter/fix-review-cache-serialization-471
Closed

Fix scan crash when framework cache hits persisted review_cache#475
MacHatter1 wants to merge 1 commit intopeteromallet:mainfrom
MacHatter1:hatter/fix-review-cache-serialization-471

Conversation

@MacHatter1
Copy link
Contributor

Problem

desloppify scan could crash while persisting state if state["review_cache"] contains framework detection / info dataclass instances (non-JSON-serializable).

Fix

  • Store JSON-safe dict payloads in review_cache for:
    • frameworks.ecosystem.present:* (framework detection)
    • framework.nextjs.info:* (Next.js info)
    • Decode these payloads back to the expected runtime dataclasses when read.
  • Harden json_default to encode dataclasses/enums defensively so future cache regressions don't brick scans.

Tests

  • pytest -q (6236 passed, 165 skipped)
  • Manual scans using workspace code via PYTHONPATH:
    • vercel/nextjs-postgres-auth-starter
    • vercel/swr
    • sindresorhus/ky

Prevent scan state persistence from crashing when review_cache contains framework detection/info dataclasses.

Adds regression coverage and keeps json_default more robust for future cache regressions.
@peteromallet
Copy link
Owner

Thanks @MacHatter1! You correctly identified the framework cache serialization crash — that's a real bug. The json_default dataclass handler part of your PR was implemented independently (commit 61bb7cb on 0.9.11), and the root cause was separately fixed by PR #483 (commit 986f884 by @maciej-trebacz) which introduces a runtime_cache to keep framework objects out of persisted state entirely.

Your encode/decode helpers for framework cache round-tripping addressed a secondary issue (cache miss after reload), but since PR #483's approach means framework objects never reach the persistence layer, that code path is no longer needed.

Appreciate the thorough investigation — you clearly dug deep into the serialization chain!

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.

2 participants