eschutho opened a new pull request, #44731:
URL: https://github.com/apache/superset/pull/44731
### SUMMARY
**Root cause**: `Slice.form_data` calls `json.loads(self.params)` and
unconditionally calls `.update()` on the result. If `params` contains valid
JSON that decodes to a non-dict type (string, array, integer, null), the parse
succeeds but `.update()` raises `AttributeError` (e.g. `'str' object has no
attribute 'update'`).
**Reachability evidence**: The Chart Create/Update API schemas define
`params` as `fields.String()` with only a `validate_json` validator — any
syntactically valid JSON is accepted and persisted, not just JSON objects.
`CreateChartCommand.__init__` already contains an `isinstance(params, dict)`
guard for its own viz_type fallback logic, confirming the codebase anticipates
non-dict `params` values but never rejects or coerces them before persisting. A
chart with `params = '"foo"'` or `params = '[1,2]'` is therefore reachable in
production.
**Fix**: `Slice.form_data` already has a `try/except Exception` around
`json.loads(self.params)` that logs "Malformed json in slice's params" and
falls back to `form_data = {}`. This PR extends that same fallback to also
cover "parsed successfully but isn't a dict" — if `json.loads` returns a
non-dict, a `ValueError` is raised into the existing exception handler,
producing the same logged warning and empty-dict fallback. The downstream
`.update()` then populates `slice_id`, `viz_type`, and `datasource` as normal.
**Why fix here**: `data_for_slices` (the dashboard dataset loader, and the
crash site from the stack trace) calls `.get()` on `slc.form_data` throughout
its loop — `Slice.form_data` is the single choke point, not the individual
callers.
### Tradeoffs
This folds "non-dict params" into the same fallback as "malformed params
JSON". A chart whose `params` column contains valid-but-non-object JSON (e.g. a
bare string or array) now silently gets default viz params (`slice_id`,
`viz_type`, `datasource`) instead of raising. This matches the existing
behavior for unparseable `params` — the chart renders with defaults rather than
crashing the entire dashboard dataset endpoint. The non-dict value is logged as
an error (same as the malformed-JSON path), so it remains observable.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — backend-only fix, no UI change.
### TESTING INSTRUCTIONS
1. Create a chart via the API with `params` set to a valid-JSON non-object,
e.g. `'"foo"'` or `'[1,2]'`.
2. Load a dashboard containing that chart.
3. **Before this fix**: `AttributeError: 'str' object has no attribute
'update'` crashes the `get_datasets` endpoint.
4. **After this fix**: The chart loads with default form data (viz_type,
datasource, slice_id populated), and a "Malformed json in slice's params" error
is logged.
**Automated regression test** added in
`tests/unit_tests/models/slice_test.py` covering JSON string, array, integer,
and null params — all 4 fail before the fix and pass after.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
