From 89cc9be9d07e2edb4bea4abb43da99927b1e7550 Mon Sep 17 00:00:00 2001 From: Mangalam Date: Thu, 6 Aug 2026 21:10:12 +0530 Subject: [PATCH] Add JSON edge-case coverage; clear parse-error messages on both sides parseJSON() in assets/profiler-engine.js and read_table() in faircode/loaders_extra.py had no test coverage for malformed or unusual JSON input, so it wasn't clear what a user actually sees when they drop a bad file (#169). - Truncated/invalid JSON syntax previously leaked a raw, engine-specific parser error (a browser SyntaxError in JS, an internal pandas ValueError in Python - and in Python's case, retried under orient="split" only to fail with an equally confusing second error). Both now fail fast with the same clear "Unsupported JSON format" wording the existing tabular-shape checks already use. - Found in the process: the JS engine's "columns orientation" check only verified each top-level value was a plain object, not that its entries were scalars, so a deeply-nested non-tabular structure like {"a": {"b": {"c": 1}}} was silently misread as one column "a" with a row "b" whose cell was the object {"c": 1}. It now throws the same clear error instead of producing a garbled table. - Added scripts/parse-json-js.js (mirrors the existing scripts/profile-json-js.js pattern) so tests can assert on parseJSON()'s error message via subprocess instead of parsing a Node stack trace. - Added tests/test_json_edge_cases.py covering all four cases named in the issue (truncated JSON, array of primitives, empty object, deeply nested structure) on both engines. Where Python's pandas-backed reader is intentionally more lenient than the JS engine (array-of-primitives, empty-object) and already returns a well-defined result rather than erroring, the test pins that existing behavior instead of changing it - loosening/tightening pandas' own JSON-orientation handling felt out of scope for a parse-error-message fix. All existing tests (including tests/test_js_parity.py) still pass. Closes #169 --- assets/profiler-engine.js | 20 ++++++- faircode/loaders_extra.py | 12 ++++ scripts/parse-json-js.js | 27 +++++++++ tests/test_json_edge_cases.py | 103 ++++++++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 scripts/parse-json-js.js create mode 100644 tests/test_json_edge_cases.py diff --git a/assets/profiler-engine.js b/assets/profiler-engine.js index 9d3e9bc..ab27393 100644 --- a/assets/profiler-engine.js +++ b/assets/profiler-engine.js @@ -140,7 +140,15 @@ function parseJSON(text) { if (typeof text !== 'string') text = String(text); if (text.charCodeAt(0) === 0xFEFF) text = text.slice(1); // strip BOM - var parsed = JSON.parse(text); // let invalid JSON syntax throw to caller + var parsed; + try { + parsed = JSON.parse(text); + } catch (syntaxErr) { + // Raw SyntaxError messages are browser-specific (e.g. "Unexpected end + // of JSON input" vs "Unexpected token") and confusing in the dropzone's + // error banner - wrap them like the tabular-shape checks below do. + throw new Error('Unsupported JSON format (not valid JSON: ' + syntaxErr.message + ').'); + } if (!parsed) return { columns: [], rows: [] }; // 1. Records format: [ { colA: 1, colB: 2 }, ... ]. Columns are the union @@ -206,7 +214,15 @@ var colNames = Object.keys(parsed); var looksColumnar = colNames.length > 0 && colNames.every(function (c) { var v = parsed[c]; - return v && typeof v === 'object' && !Array.isArray(v); + if (!v || typeof v !== 'object' || Array.isArray(v)) return false; + // Each entry must be a scalar (index -> value), not itself a nested + // object - otherwise a deeply-nested, non-tabular structure like + // {"a": {"b": {"c": 1}}} is silently misread as one column "a" with + // a row "b" whose cell value is the object {"c": 1}. + return Object.keys(v).every(function (k) { + var cell = v[k]; + return cell === null || typeof cell !== 'object'; + }); }); if (looksColumnar) { var indexKeys = []; diff --git a/faircode/loaders_extra.py b/faircode/loaders_extra.py index 7d64057..00cabd0 100644 --- a/faircode/loaders_extra.py +++ b/faircode/loaders_extra.py @@ -11,6 +11,7 @@ from __future__ import annotations +import json from pathlib import Path import pandas as pd @@ -22,6 +23,17 @@ def read_table(path: str) -> pd.DataFrame: suffix = Path(path).suffix.lower() if suffix == ".json": + with open(path, "r", encoding="utf-8") as f: + raw = f.read() + try: + json.loads(raw) + except json.JSONDecodeError as exc: + # pandas' own parser error for malformed JSON (e.g. a truncated + # file) is an internal/version-specific message, and calling + # pd.read_json() a second time with orient="split" below would + # just fail again with an equally confusing one. Fail fast with + # a clear message instead, mirroring the JS engine's parseJSON(). + raise ValueError(f"Unsupported JSON format (not valid JSON: {exc}).") from exc try: return pd.read_json(path) except ValueError: diff --git a/scripts/parse-json-js.js b/scripts/parse-json-js.js new file mode 100644 index 0000000..c0cd591 --- /dev/null +++ b/scripts/parse-json-js.js @@ -0,0 +1,27 @@ +#!/usr/bin/env node + +const fs = require("fs"); +const path = require("path"); + +// Load the profiler engine. It registers itself on globalThis.FairCodeProfiler. +require(path.join(__dirname, "..", "assets", "profiler-engine.js")); + +const E = globalThis.FairCodeProfiler; + +if (process.argv.length !== 3) { + console.error("Usage: node scripts/parse-json-js.js "); + process.exit(1); +} + +const jsonPath = process.argv[2]; +const text = fs.readFileSync(jsonPath, "utf8"); + +// Report parseJSON()'s outcome as JSON on stdout (exit 0 either way) so +// callers - namely tests/test_json_edge_cases.py - can assert on the error +// message for malformed input without having to parse a Node stack trace. +try { + const table = E.parseJSON(text); + process.stdout.write(JSON.stringify({ ok: true, table: table })); +} catch (e) { + process.stdout.write(JSON.stringify({ ok: false, error: e.message })); +} diff --git a/tests/test_json_edge_cases.py b/tests/test_json_edge_cases.py new file mode 100644 index 0000000..de63658 --- /dev/null +++ b/tests/test_json_edge_cases.py @@ -0,0 +1,103 @@ +"""Edge-case coverage for malformed/unusual JSON input on both the Python +CLI (faircode.loaders_extra.read_table) and the browser profiler engine's +parseJSON() (#169). + +Run from the repo root: pytest tests/ -q +""" + +import json +import subprocess +from pathlib import Path + +import pytest + +from faircode.loaders_extra import read_table + +REPO_ROOT = Path(__file__).resolve().parent.parent + + +def _run_js_parse(tmp_path, text): + path = tmp_path / "input.json" + path.write_text(text, encoding="utf-8") + completed = subprocess.run( + ["node", "scripts/parse-json-js.js", str(path)], + cwd=REPO_ROOT, + capture_output=True, + text=True, + encoding="utf-8", + check=True, + ) + return json.loads(completed.stdout) + + +def _write_json(tmp_path, text): + path = tmp_path / "input.json" + path.write_text(text, encoding="utf-8") + return path + + +# ── Truncated / syntactically invalid JSON ─────────────────────────────────── +# Both engines used to leak their underlying parser's raw message here (a +# browser-specific SyntaxError in JS, an equally internal pandas ValueError +# in Python). Both now fail fast with a clear, consistent message. +def test_truncated_json_js_gives_clear_error(tmp_path): + result = _run_js_parse(tmp_path, '{"a": 1, "b":') + assert result["ok"] is False + assert "Unsupported JSON format" in result["error"] + + +def test_truncated_json_python_gives_clear_error(tmp_path): + path = _write_json(tmp_path, '{"a": 1, "b":') + with pytest.raises(ValueError, match="Unsupported JSON format"): + read_table(str(path)) + + +# ── JSON array of primitives, e.g. [1, 2, 3] ───────────────────────────────── +def test_array_of_primitives_js_gives_clear_error(tmp_path): + result = _run_js_parse(tmp_path, "[1, 2, 3]") + assert result["ok"] is False + assert "Unsupported JSON format" in result["error"] + + +def test_array_of_primitives_python_current_behavior(tmp_path): + # pandas' read_json treats a bare array of primitives as a single-column + # table (column "0"). This differs from the JS engine's stricter check + # above; pinning it here documents the existing behavior rather than + # changing it, since loosening/tightening pandas' own JSON orientation + # handling is out of scope for this fix. + path = _write_json(tmp_path, "[1, 2, 3]") + df = read_table(str(path)) + assert df.to_dict(orient="list") == {0: [1, 2, 3]} + + +# ── Empty object {} ─────────────────────────────────────────────────────────── +def test_empty_object_js_gives_clear_error(tmp_path): + result = _run_js_parse(tmp_path, "{}") + assert result["ok"] is False + assert "Unsupported JSON format" in result["error"] + + +def test_empty_object_python_current_behavior(tmp_path): + path = _write_json(tmp_path, "{}") + df = read_table(str(path)) + assert df.empty + + +# ── Deeply-nested, non-tabular structure ───────────────────────────────────── +def test_deeply_nested_json_js_gives_clear_error(tmp_path): + # {"a": {"b": {"c": 1}}} used to slip past the JS engine's "columns + # orientation" check (which only verified each top-level value was a + # plain object, not that its entries were scalars) and get silently + # misread as one column "a" with a row "b" whose cell is {"c": 1}. + result = _run_js_parse(tmp_path, json.dumps({"a": {"b": {"c": 1}}})) + assert result["ok"] is False + assert "Unsupported JSON format" in result["error"] + + +def test_deeply_nested_json_python_current_behavior(tmp_path): + # pandas' read_json has no equivalent guard and happily returns a + # DataFrame with a dict as a cell value. Pinning current behavior here; + # not changed by this fix (see array-of-primitives note above). + path = _write_json(tmp_path, json.dumps({"a": {"b": {"c": 1}}})) + df = read_table(str(path)) + assert df.to_dict(orient="records") == [{"a": {"c": 1}}]