Skip to content

fix: JSONConverter raises KeyError on ByteStream sources without file_path - #12208

Merged
julian-risch merged 1 commit into
deepset-ai:mainfrom
mittalpk:fix/json-converter-bytestream-keyerror
Aug 4, 2026
Merged

julian-risch merged 1 commit into
deepset-ai:mainfrom
mittalpk:fix/json-converter-bytestream-keyerror

Conversation

@mittalpk

Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes

All three exception handlers in _get_content_and_meta() — invalid UTF-8 decoding, a jq_schema filter that fails to apply, and malformed JSON — log a warning using source.meta["file_path"], an unconditional dict-key access. A bare ByteStream (e.g. ByteStream.from_string(...), the exact pattern shown in this component's own docstring usage examples) has empty meta by default, so on any of these three error paths the error-handling code itself raises KeyError: 'file_path', masking the real error (UnicodeDecodeError, filter failure, or JSONDecodeError) and crashing run() entirely instead of logging a warning and skipping that one source, which is the documented/intended behavior.

Two prior merged PRs (#8775, #11980) touched this exact function, but both only tested via file-path sources, which auto-populate file_path in meta — so this bug survived both.

Fixed by using source.meta.get("file_path", "unknown") at all three call sites, consistent with how run() itself already safely does bytestream.meta.get("file_path") a few lines below.

How did you test it?

Reproduced the crash directly against main (JSONConverter(content_key="text").run(sources=[ByteStream(data=b"\xff\xfe not valid utf-8")])), confirmed the KeyError.

Added three new tests, one per exception path, each using a bare ByteStream mirroring the repo's existing file-path-based tests for the same three paths (test_run_with_bad_filter, test_run_with_bad_encoding, test_run_with_malformed_json_and_content_key). Confirmed all three fail on unpatched json.py (git stash) with the same KeyError, and pass after the fix. Ran the full converters unit suite (hatch run test:unit test/components/converters/) — 265 passed, no regressions. hatch run test:types — no issues. hatch run fmt — clean.

Notes for the reviewer

Same shape of bug and same root cause as #12207 (MSGToDocument), found in the same audit pass — flagging in case a reviewer wants to look at both together, though they're independent files/components and can be merged separately.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have added unit tests and updated the docstrings.
  • I've used a conventional commit type (fix:) for the PR title.
  • I have added a release note file.
  • I have run hatch run fmt / hatch run test:types / hatch run test:unit and fixed any issue.

This PR was developed with AI-assisted tooling (Claude Code); I reviewed the diagnosis, the fix, and the tests, and ran the test suite and quality checks locally as noted above.

…_path

All three exception handlers in _get_content_and_meta() (invalid UTF-8,
a failing jq_schema filter, malformed JSON) log a warning using
source.meta["file_path"] -- an unconditional dict-key access. A bare
ByteStream (e.g. ByteStream.from_string(...), the exact pattern shown
in this component's own docstring examples) has no such key, so the
error-handling code itself raised KeyError, masking the real error and
crashing run() instead of skipping that one source as intended.

Fixed by using source.meta.get("file_path", "unknown") at all three
sites.
@mittalpk
mittalpk requested a review from a team as a code owner July 31, 2026 07:53
@mittalpk
mittalpk requested review from julian-risch and removed request for a team July 31, 2026 07:53
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

@mittalpk is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant

CLAassistant commented Jul 31, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@julian-risch julian-risch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me! Thank you for your contribution!

@julian-risch
julian-risch enabled auto-merge (squash) August 4, 2026 06:54
@github-actions github-actions Bot added the type:documentation Improvements on the docs label Aug 4, 2026
@julian-risch
julian-risch merged commit 1453504 into deepset-ai:main Aug 4, 2026
25 of 26 checks passed
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/converters
  json.py
  msg.py
  utils.py
  haystack/components/preprocessors
  recursive_splitter.py
  haystack/dataclasses
  document.py
Project Total  

This report was generated by python-coverage-comment-action

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

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants