Skip to content

fix(chart): add error logging to /api/v1/chart/data endpoint#38744

Draft
rebenitez1802 wants to merge 1 commit intomasterfrom
rebenitez1802/fix/add-debug-logs-to-chart-data-endpoint
Draft

fix(chart): add error logging to /api/v1/chart/data endpoint#38744
rebenitez1802 wants to merge 1 commit intomasterfrom
rebenitez1802/fix/add-debug-logs-to-chart-data-endpoint

Conversation

@rebenitez1802
Copy link
Copy Markdown
Contributor

@rebenitez1802 rebenitez1802 commented Mar 19, 2026

SUMMARY

Add error-level logging to all failure paths in the chart data API (/api/v1/chart/data) to help diagnose intermittent 400 BAD REQUEST failures during CSV exports.

TESTING INSTRUCTIONS

  1. Trigger a CSV export from a dashboard — should succeed with no extra logs
  2. Simulate failures (malformed form_data, missing datasource, permission denied) — error logs should appear with request context
  3. Run existing chart data API tests — no regressions expected

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@github-actions github-actions bot added the api Related to the REST API label Mar 19, 2026
Add error-level logging to all failure paths in the chart data API
to help diagnose intermittent 400 BAD REQUEST failures during CSV
exports. Previously, JSON parsing errors were silently swallowed by
contextlib.suppress and validation/query errors returned 400 without
any logging, making it impossible to identify which failure path was
hit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rebenitez1802 rebenitez1802 force-pushed the rebenitez1802/fix/add-debug-logs-to-chart-data-endpoint branch from c21321d to d15f781 Compare March 19, 2026 15:11
request.content_length,
bool(request.form.get("form_data")),
request.referrer,
)
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.

When form_data is present but unparseable, this block fires after the except above has already logged. A single parse failure produces two logger.error entries in the log, which could be confusing when correlating events.

@bito-code-review
Copy link
Copy Markdown
Contributor

Yes, the code change introduces duplicate logging for unparseable form_data. The except block logs an error on JSONDecodeError, and then the subsequent if json_body is None block logs another error, leading to two entries for a single failure.

superset/charts/data/api.py

try:
                json_body = json.loads(request.form["form_data"])
            except (TypeError, json.JSONDecodeError):
                logger.error(
                    "Failed to parse form_data JSON: "
                    "content_type=%s, content_length=%s, form_data_length=%s, "
                    "referrer=%s",
                    request.content_type,
                    request.content_length,
                    len(request.form.get("form_data", "")),
                    request.referrer,
                )
            if json_body is None:
                logger.error(
                    "Chart data request rejected: json_body is None. "
                    "is_json=%s, content_type=%s, content_length=%s, "
                    "has_form_data=%s, referrer=%s",
                    request.is_json,
                    request.content_type,
                    request.content_length,
                    bool(request.form.get("form_data")),
                    request.referrer,
                )
                return self.response_400(message=_("Request is not JSON"))

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

Labels

api Related to the REST API review:draft size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants