Skip to content

Return client errors for malformed Chart.js configurations - #222

Open
typpo wants to merge 1 commit into
masterfrom
fix/chart-config-validation
Open

typpo wants to merge 1 commit into
masterfrom
fix/chart-config-validation

Conversation

@typpo

@typpo typpo commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Malformed Chart.js configurations currently become HTTP 500 responses with stack-trace logs and empty X-quickchart-error headers. This turns ordinary input mistakes into operational noise.

The exact Cannot create property 'datasets' on string failure is reproducible with {type: 'bar', data: 'graph TD; A --> B'}. A top-level expression evaluating to a string fails earlier while QuickChart applies defaults, and raw Mermaid fails JavaScript evaluation. All three returned 500 on the unchanged base.

This change:

  • Validates the root configuration and structural containers before QuickChart mutates them, without JSON-roundtripping callbacks or imposing a schema on plugin-specific dataset values.
  • Checks effective dataset types against the selected Chart.js registry after user plugins can register controllers. Rejected instances are destroyed so type-validation failures do not retain charts/canvases.
  • Uses a dedicated ChartConfigError for known input failures. GET/POST PNG, SVG, and PDF responses return 400 with a useful message; logging emits a fixed info event without the submitted input or exception stack.
  • Preserves unexpected renderer failures as 500 with an error-stack log. It deliberately does not classify every TypeError or ReferenceError as bad input.
  • Resolves the Chart helper before evaluating JavaScript configurations, preserves computed configs/callbacks/custom controllers, and escapes evaluation-error text in SVG responses.
  • Handles missing configurations and non-string base64 payloads as 400.

Validation:

  • Unchanged base 9c31a0b: 38 existing tests passing.
  • Patched yarn test: 102 passing (64 new regression cases).
  • Real rendering tested across Chart.js 2/3/4, including callbacks, computed arrays, mixed charts, a custom controller, gradient fills, object-map data, PNG/SVG/PDF error responses, instance cleanup, logging, and retained 500s.
  • Formatting checks for new files and git diff --check pass.
  • Tests ran on Linux with Node 16.20.2, the pinned lockfile, and canvas 2.9.3. Node 18.16.0 was attempted first, but canvas 2.9.3 has no downloadable Node 18 binary and native build prerequisites could not be installed in the execution environment. The Node 18 Docker/runtime check remains outstanding; no dependency files were changed.

Deployment scope: this PR changes the public OSS repository only. It does not establish that a hosted service uses this commit or these dependencies. Before applying it to another deployment, identify the immutable image and source commit, compare its renderer/error middleware, and verify 400 responses plus stack-free validation logging in staging. The public renderer already disables Chart.js animation; an animation-origin window is not defined error needs its own reproduction and should not be hidden by a blanket exception rule.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant