Skip to content

fix: preserve JSON scalar input types in local storage - #1243

Open
peterolkhov wants to merge 1 commit into
apify:masterfrom
peterolkhov:codex/preserve-local-json-input-types
Open

peterolkhov wants to merge 1 commit into
apify:masterfrom
peterolkhov:codex/preserve-local-json-input-types

Conversation

@peterolkhov

Copy link
Copy Markdown

A pre-existing INPUT.json or valid JSON INPUT file in local storage gets its metadata inferred from the parsed value. Scalars then read back as plain text or bytes, so false, 42, and null lose their JSON types.

This keeps successful JSON parsing as the source of the content type. The tests cover both filenames, scalar and falsy values, object/array controls, unchanged input bytes, explicit metadata, and malformed JSON. Existing metadata is respected; this does not rewrite sidecars created earlier.

Tested: uv run poe check-code (lint, type check, 767 unit tests) and the focused filesystem/Actor/storage tests (75 passed).

Prepared with Codex assistance.

@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Nov 9, 2026
@janbuchar
janbuchar self-requested a review November 10, 2026 09:07
value = content

content_type = infer_mime_type(value)
content_type = 'application/json; charset=utf-8' if is_json else infer_mime_type(value)

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.

value is now used only to choose the content type, and the .json suffix check appears twice. Setting content_type in each branch removes the is_json flag and infer_mime_type, with the same result:

if record_path.suffix == '.json':
    json.loads(content.decode('utf-8'))  # Raises on malformed JSON.
    content_type = 'application/json; charset=utf-8'
elif record_path.suffix == '.txt':
    content_type = 'text/plain; charset=utf-8'
elif record_path.suffix == '':
    try:
        json.loads(content.decode('utf-8'))
    except (json.JSONDecodeError, UnicodeDecodeError):
        content_type = 'application/octet-stream'
    else:
        content_type = 'application/json; charset=utf-8'
else:
    content_type = 'application/octet-stream'

✍️ Drafted by Claude Code

@@ -136,10 +137,12 @@ async def _create_missing_metadata_for_input_file(key: str, record_path: Path) -
value = json.loads(content.decode('utf-8'))
except json.JSONDecodeError:

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.

This bug is older than the PR. A bare INPUT with non-UTF-8 bytes raises UnicodeDecodeError from open(). _actor._read_input_file catches both errors and returns the bytes, so this path should do the same.

Suggested change
except json.JSONDecodeError:
except (json.JSONDecodeError, UnicodeDecodeError):

✍️ Drafted by Claude Code

pytest.param(0.0, id='zero-float'),
pytest.param(True, id='true'),
pytest.param(False, id='false'),
pytest.param(None, id='null'),

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.

Nothing tests a bare INPUT that is not JSON (for example b'hello', and a binary payload if the UnicodeDecodeError fix goes in). It should keep application/octet-stream and read back as bytes. A separate test fits better, because this one writes json.dumps(...).

✍️ Drafted by Claude Code

[pytest.param('INPUT', id='bare'), pytest.param('INPUT.json', id='json')],
)
@pytest.mark.parametrize(
'pre_existing_input',

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.

13 values x 2 file names gives 26 cases. One object, 0, False, None, '' and 1.5 cover the same paths, including the falsy and bool/int cases.

✍️ Drafted by Claude Code

This branch has not been deployed

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

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants