From e76890b5b9eea650d1acdac9609bc3ab6704662b Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 27 May 2026 14:39:35 +0200 Subject: [PATCH 1/2] Wrap corrupt cache reads in CacheCorruptError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FileCache#read leaks JSON::ParserError and KeyError to callers when the cache file is malformed (truncated, hand-edited, partial write). The original message gives no hint that the file is FixtureKit's cache or that the fix is to delete and regenerate. Wrap both error classes in a new FixtureKit::CacheCorruptError that includes the cache path and the original error class/message, plus a short note pointing at the remediation. This is a strict superset of the previous behavior — a rescue on the new error class still catches malformed cache files, and StandardError catches everything as before. No retry, no auto-regeneration. Disk-vs-memory consistency bugs should be loud, not silently papered over. --- lib/fixture_kit.rb | 1 + lib/fixture_kit/file_cache.rb | 4 ++++ spec/unit/file_cache_spec.rb | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/lib/fixture_kit.rb b/lib/fixture_kit.rb index 7caf094..33557a1 100644 --- a/lib/fixture_kit.rb +++ b/lib/fixture_kit.rb @@ -6,6 +6,7 @@ class DuplicateNameError < Error; end class InvalidFixtureDeclaration < Error; end class MultipleFixtures < Error; end class CacheMissingError < Error; end + class CacheCorruptError < Error; end class FixtureDefinitionNotFound < Error; end class RunnerAlreadyStartedError < Error; end class CircularFixtureInheritance < Error; end diff --git a/lib/fixture_kit/file_cache.rb b/lib/fixture_kit/file_cache.rb index 51da248..b58bd91 100644 --- a/lib/fixture_kit/file_cache.rb +++ b/lib/fixture_kit/file_cache.rb @@ -33,6 +33,10 @@ def read end MemoryCache.new(data: data, exposed: exposed) + rescue JSON::ParserError, KeyError => e + raise FixtureKit::CacheCorruptError, + "FixtureKit cache file at #{path} is corrupt or malformed (#{e.class}: #{e.message}). " \ + "Delete it and re-run to regenerate." end def write(data) diff --git a/spec/unit/file_cache_spec.rb b/spec/unit/file_cache_spec.rb index e3415e8..0342548 100644 --- a/spec/unit/file_cache_spec.rb +++ b/spec/unit/file_cache_spec.rb @@ -83,6 +83,24 @@ expect(File.exist?(nested_path)).to be(true) end + + it "raises CacheCorruptError when the file contains invalid JSON" do + FileUtils.mkdir_p(cache_path) + File.write(file_path, "this is not json") + + expect { file_cache.read }.to raise_error(FixtureKit::CacheCorruptError) do |error| + expect(error.message).to include(file_path) + expect(error.message).to include("JSON::ParserError") + end + end + + it "raises CacheCorruptError when the JSON is missing required keys" do + FileUtils.mkdir_p(cache_path) + File.write(file_path, JSON.dump({ "data" => {} })) # no "exposed" key + + expect { file_cache.read } + .to raise_error(FixtureKit::CacheCorruptError, /is corrupt or malformed.*KeyError/) + end end describe "#serialize_exposed" do From c26bd340a174a03b7ffbfc3ac7a2690de9fedb76 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 3 Jun 2026 14:34:16 +0200 Subject: [PATCH 2/2] Localize corrupt-cache rescue to #parse and add CacheCorruptError.for Scope the rescue to JSON parsing and required-key validation in a new private #parse, so decode-stage errors (e.g. an unregistered coder's KeyError from #coder_for) are no longer mislabeled as a corrupt cache file. Move the message into CacheCorruptError via a .for(path, cause) factory, which keeps the standard raise idiom intact. Document CacheCorruptError in the reference. Addresses review feedback from @ngan on #62. --- docs/reference.md | 1 + lib/fixture_kit.rb | 9 ++++++++- lib/fixture_kit/file_cache.rb | 19 ++++++++++++++----- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 4a78518..20697be 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -319,6 +319,7 @@ Public error classes: - `FixtureKit::InvalidFixtureDeclaration` - `FixtureKit::MultipleFixtures` - `FixtureKit::CacheMissingError` +- `FixtureKit::CacheCorruptError` - `FixtureKit::FixtureDefinitionNotFound` - `FixtureKit::RunnerAlreadyStartedError` - `FixtureKit::CircularFixtureInheritance` diff --git a/lib/fixture_kit.rb b/lib/fixture_kit.rb index 33557a1..f84cd14 100644 --- a/lib/fixture_kit.rb +++ b/lib/fixture_kit.rb @@ -6,7 +6,14 @@ class DuplicateNameError < Error; end class InvalidFixtureDeclaration < Error; end class MultipleFixtures < Error; end class CacheMissingError < Error; end - class CacheCorruptError < Error; end + class CacheCorruptError < Error + def self.for(path, cause) + new( + "FixtureKit cache file at #{path} is corrupt or malformed " \ + "(#{cause.class}: #{cause.message}). Delete it and re-run to regenerate." + ) + end + end class FixtureDefinitionNotFound < Error; end class RunnerAlreadyStartedError < Error; end class CircularFixtureInheritance < Error; end diff --git a/lib/fixture_kit/file_cache.rb b/lib/fixture_kit/file_cache.rb index b58bd91..8730760 100644 --- a/lib/fixture_kit/file_cache.rb +++ b/lib/fixture_kit/file_cache.rb @@ -17,7 +17,7 @@ def exists? end def read - content = JSON.parse(File.read(path)) + content = parse data = content.fetch("data").to_h do |coder_name, coder_data| coder = coder_for(coder_name) @@ -33,10 +33,6 @@ def read end MemoryCache.new(data: data, exposed: exposed) - rescue JSON::ParserError, KeyError => e - raise FixtureKit::CacheCorruptError, - "FixtureKit cache file at #{path} is corrupt or malformed (#{e.class}: #{e.message}). " \ - "Delete it and re-run to regenerate." end def write(data) @@ -64,6 +60,19 @@ def serialize_exposed(exposed) private + # Reads and parses the cache file, validating that the required top-level + # keys are present. The rescue is scoped to just this step so that decode + # errors raised later in #read (e.g. an unregistered coder, a configuration + # error) are not misreported as a corrupt cache file. + def parse + content = JSON.parse(File.read(path)) + content.fetch("data") + content.fetch("exposed") + content + rescue JSON::ParserError, KeyError => e + raise FixtureKit::CacheCorruptError.for(path, e) + end + def coder_for(class_name) @coder_for ||= FixtureKit.runner.coders.index_by { |c| c.class.name } @coder_for.fetch(class_name)