fix: mssql json issue#322
Conversation
📝 WalkthroughWalkthroughMSSQL row conversion now attempts to decode string values beginning with ChangesMSSQL JSON parsing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py (1)
87-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for the row-conversion contract.
Cover valid objects and arrays, surrounding whitespace, malformed JSON preservation, ordinary strings, deeply nested input, and the no-result-set path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py` around lines 87 - 115, Add regression tests covering MSSQLClient._maybe_parse_json for valid objects, arrays, surrounding whitespace, malformed JSON, ordinary strings, and deeply nested JSON, asserting non-JSON or invalid text is preserved. Add a test for MSSQLClient._rows_as_dicts when cursor.description is absent, confirming it returns an empty list without fetching rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py`:
- Around line 96-99: Update the best-effort JSON parsing exception handler in
the surrounding row-conversion logic to catch RecursionError alongside
ValueError and TypeError, returning the original value so one deeply nested or
malformed database field cannot abort _rows_as_dicts(). Add a regression test
covering a deeply nested JSON string and verify conversion continues
successfully.
- Around line 109-114: Update the row-to-dictionary comprehension in the MSSQL
result conversion to use strict zipping between columns and each row, so
cardinality mismatches raise ValueError instead of truncating data. Keep the
existing _maybe_parse_json processing and fetchall flow unchanged.
---
Nitpick comments:
In `@wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py`:
- Around line 87-115: Add regression tests covering
MSSQLClient._maybe_parse_json for valid objects, arrays, surrounding whitespace,
malformed JSON, ordinary strings, and deeply nested JSON, asserting non-JSON or
invalid text is preserved. Add a test for MSSQLClient._rows_as_dicts when
cursor.description is absent, confirming it returns an empty list without
fetching rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c4752f39-b5f0-4043-af6b-c716f550cbd7
📒 Files selected for processing (1)
wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py
| try: | ||
| return json.loads(stripped) | ||
| except (ValueError, TypeError): | ||
| return value |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)server/packages/flo_cloud/flo_cloud/mssql/mssql\.py$' || true
echo "== file excerpt =="
if [ -f wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py ]; then
nl -ba wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py | sed -n '1,180p'
fi
echo "== search helper usages/definitions =="
rg -n "json\.loads|def .*json|RecursionError|ValueError|TypeError" wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py || true
echo "== deterministic RecursionError probe =="
python3 - <<'PY'
import json, sys
nested = {"a": None}
depth = getattr(sys, "getrecursionlimit", lambda: 1000)()
try:
for _ in range(1_000):
nested = {"a": nested}
except RecursionError as e:
print("constructed_depth", depth, "constructed_recursion_limit_breach", type(e).__name__)
try:
json.loads(json.dumps(nested))
except RecursionError as e2:
print("loads_raises_recursion_error", type(e2).__name__)
except Exception as e2:
print("loads_raises_else", type(e2).__name__, e2)
PYRepository: rootflo/wavefront
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt with awk =="
awk 'NR<=180 {printf "%6d\t%s\n", NR, $0}' wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py
echo "== search helper usages/definitions =="
grep -nE "json\.loads|def .*json|RecursionError|ValueError|TypeError" wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py || true
echo "== deterministic RecursionError probe =="
python3 - <<'PY'
import json, sys
nested = {"a": None}
depth = getattr(sys, "getrecursionlimit", lambda: 1000)()
try:
for _ in range(1_000):
nested = {"a": nested}
except RecursionError as e:
print("constructed_depth", depth, "constructed_recursion_limit_breach", type(e).__name__)
try:
json.loads(json.dumps(nested))
except RecursionError as e2:
print("loads_raises_recursion_error", type(e2).__name__)
except Exception as e2:
print("loads_raises_else", type(e2).__name__, e2)
PYRepository: rootflo/wavefront
Length of output: 8144
🌐 Web query:
Python json.loads RecursionError for deeply nested objects
💡 Result:
In Python, the json.loads() and json.dump() functions use recursion to process nested JSON structures [1][2]. When the depth of these structures exceeds the interpreter's recursion limit, Python raises a RecursionError [1][3]. Key details regarding this behavior include: * Cause: The json module's parser functions (specifically those in the C implementation) must track nesting levels to correctly parse objects and arrays [2]. Once the depth of the nesting reaches the limit configured by sys.getrecursionlimit(), the interpreter halts the operation to prevent a stack overflow [2][3][4]. * Documentation: While RecursionError is a standard Python exception, it is often not explicitly listed in the json module documentation as a potential error for loads() or dumps(), as it is considered an implementation-level safeguard rather than a typical user-facing error [1]. * Mitigation and Handling: * Avoid Excessive Nesting: If you control the data source, consider flattening or restructuring the data before transmission [5]. * sys.setrecursionlimit(): You can technically increase the limit using sys.setrecursionlimit(), but this is generally discouraged for solving deep JSON nesting issues because it risks crashing the interpreter with a segmentation fault if the C stack limit is reached before the Python recursion limit [3]. * Alternative Libraries: If you frequently work with extremely deep or complex JSON, consider using third-party libraries (e.g., orjson, ujson) which often implement iterative or alternative parsing strategies that may handle deeper nesting more efficiently than the standard library [4]. If you are encountering this error when processing nested data in your code, you should typically address it by refactoring your data structure or adopting a stream-based or iterative parsing approach if the data format allows [5]. Using libraries like pandas.json_normalize can also assist in managing semi-structured or deeply nested data once it has been successfully parsed [6][7].
Citations:
- 1: json.load() can raise undocumented RecursionError python/cpython#135835
- 2: https://bugs.python.org/issue12017
- 3: Regression in 3.12 beta in json.dump deeply nested dict python/cpython#107263
- 4: Use RecursionError to check for circular references in json.dumps python/cpython#134935
- 5: https://thelinuxcode.com/how-to-parse-nested-json-in-python-with-practical-patterns-that-hold-up-in-2026/
- 6: https://www.geeksforgeeks.org/python/how-to-parse-nested-json-in-python/
- 7: https://pandas.pydata.org/docs/reference/api/pandas.json_normalize.html?highlight=json_normalize
Keep best-effort JSON parsing non-fatal for deeply nested input.
json.loads can raise RecursionError on excessively nested JSON, and _rows_as_dicts() will abort conversion of the whole result set from one malformed database value. Catch RecursionError alongside ValueError/TypeError and add a regression test for a deeply nested JSON string.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py` around lines 96
- 99, Update the best-effort JSON parsing exception handler in the surrounding
row-conversion logic to catch RecursionError alongside ValueError and TypeError,
returning the original value so one deeply nested or malformed database field
cannot abort _rows_as_dicts(). Add a regression test covering a deeply nested
JSON string and verify conversion continues successfully.
| return [ | ||
| { | ||
| col: MSSQLClient._maybe_parse_json(value) | ||
| for col, value in zip(columns, row) | ||
| } | ||
| for row in cursor.fetchall() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching mssql.py:"
fd -a 'mssql.py$' . || true
echo
echo "Python files matching pyproject/config:"
fd -a '(^pyproject\.toml$|^setup\.py$|^setup\.cfg$|^tox\.ini$|(^|/)\.pre-commit-config\.yaml$)' . || true
echo
echo "Inspect target file outline and relevant lines:"
FILE=$(fd 'mssql.py$' . | head -n 1 || true)
if [ -n "${FILE:-}" ]; then
echo "FILE=$FILE"
wc -l "$FILE"
sed -n '1,180p' "$FILE" | nl -ba
fi
echo
echo "Search for zip strict support / Python target indicators:"
rg -n "python_requires|requires-python|min_python|python_version|zip\\(" -S . || true
echo
echo "Check Python installed and strict zip behavior:"
python3 - <<'PY'
import sys
print(sys.version)
try:
list(zip([1], [1, 2], strict=True))
except Exception as e:
print(type(e).__name__, str(e))
else:
print("no exception")
PYRepository: rootflo/wavefront
Length of output: 2612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect target file relevant lines:"
sed -n '1,180p' wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py | sed 's/^/ /'
echo
echo "Relevant pyproject/package target indicators:"
sed -n '1,220p' wavefront/server/pyproject.toml
echo "---- flo_cloud pyproject ----"
sed -n '1,180p' wavefront/server/packages/flo_cloud/pyproject.toml
echo
echo "Search target/config indicators and zip usage:"
rg -n "python_requires|requires-python|min_python|python-version|python_version|zip\\(|pyupgrade|ruff|strict=True" -S wavefront/server/pyproject.toml wavefront/server/packages/flo_cloud/pyproject.toml . || true
echo
echo "Python installed and strict zip behavior:"
python3 - <<'PY'
import sys
print("python", sys.version)
try:
result = list(zip(["col"], ["a", "b"], strict=True))
except Exception as e:
print("strict_zip_exception", type(e).__name__, str(e))
else:
print("strict_zip_result", result)
try:
result = list(zip(["col"], ["a", "b"], strict=False))
except Exception as e:
print("no-strict-exception", type(e).__name__)
else:
print("no-strict_result", result)
PYRepository: rootflo/wavefront
Length of output: 14399
🌐 Web query:
Python 3.11 zip strict parameter documentation ValueError differing length
💡 Result:
In Python 3.11, the built-in zip function includes a strict parameter [1] (introduced in Python 3.10 via PEP 618 [2][3]). When strict=True is passed, zip raises a ValueError if the provided iterables have differing lengths [1][3]. Key details regarding this behavior include: 1. Mechanism: The strict option does not perform an explicit len check on the inputs [4]. Instead, it ensures that all iterables are exhausted simultaneously [4]. If one iterable is exhausted while others still have items, zip raises a ValueError [2][4]. 2. Timing of Error: Because zip is an iterator, the ValueError is raised at the exact point of exhaustion [2][4]. If you are iterating over the zip object (e.g., in a for loop), the loop body will successfully execute for all matching pairs before the exception is raised [4]. 3. Error Message: The resulting ValueError provides a descriptive message indicating which argument is longer or shorter than the others [1][2][3]. This feature is intended to help prevent bugs where iterables are assumed to be of equal length but are not, ensuring that data is not silently ignored as it is with the default strict=False behavior [1][3][5].
Citations:
- 1: https://docs.python.org/release/3.11.1/library/functions.html
- 2: https://peps.python.org/pep-0618/
- 3: https://realpython.com/python-zip-function/
- 4: Unclear documentation of
zip()'sstrictoption python/cpython#100101 - 5: python/cpython@59cf853
Make column/row cardinality mismatches fail loudly.
The project targets Python 3.11, so zip(columns, row, strict=True) raises a ValueError when a row length differs from cursor.description, preventing silently truncated dictionaries.
Proposed fix
- for col, value in zip(columns, row)
+ for col, value in zip(columns, row, strict=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return [ | |
| { | |
| col: MSSQLClient._maybe_parse_json(value) | |
| for col, value in zip(columns, row) | |
| } | |
| for row in cursor.fetchall() | |
| return [ | |
| { | |
| col: MSSQLClient._maybe_parse_json(value) | |
| for col, value in zip(columns, row, strict=True) | |
| } | |
| for row in cursor.fetchall() |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 112-112: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wavefront/server/packages/flo_cloud/flo_cloud/mssql/mssql.py` around lines
109 - 114, Update the row-to-dictionary comprehension in the MSSQL result
conversion to use strict zipping between columns and each row, so cardinality
mismatches raise ValueError instead of truncating data. Keep the existing
_maybe_parse_json processing and fetchall flow unchanged.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit