You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/07/22 15:49:05 UTC
(superset) 01/01: fix(echarts): guard timeseries annotation + tooltip render against missing data (sc-114915)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch bug-114915-echarts-timeseries-crash
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 811137083a3c21b018d24ee6e6b414876f260fbb
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Wed Jul 22 15:48:49 2026 +0000
fix(echarts): guard timeseries annotation + tooltip render against missing data (sc-114915)
Line/Bar dashboards crashed at render for customer 2cad with
"Cannot read properties of undefined (reading '__ec_inner_45')" and
"Cannot read properties of null (reading '5')".
Root cause (confirmed regression): PR #34709 (fc95c4fc89, 2025-08-15)
replaced the null-tolerant guard `isTimeseriesAnnotationResult(result)`
(Array.isArray, false for undefined) with an unguarded
`const { records } = result` in `transformTimeseriesAnnotation`. When a
shown Timeseries annotation layer's backing data is missing
(annotationData[name] undefined because the source was deleted/renamed or
filtered to empty), the destructure throws and aborts the entire chart
render. It's a latent trap armed by #34709 and sprung by a data/config
shape, which is why it surfaced on a stable build with no redeploy.
Fixes:
- transformTimeseriesAnnotation: `const records = result?.records` (H1),
mirroring the safe sibling `extractRecordAnnotations`.
- Defensive null-guards in the render/hover formatters that index into
potentially null/empty series (H3): timeseries tooltip formatter,
series label formatter, extractForecastValuesFromTooltipParams,
extractTooltipKeys.
- Stream-stack baseline hardening for empty/ragged series (H4):
getBaselineSeriesForStream and the stream `.data.map` in transformProps.
Adds regression tests covering each shape (missing annotation data,
all-null/empty tooltip params, ragged/empty stream series). The new tests
fail before this change and pass after.
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
.../src/Timeseries/transformProps.ts | 15 ++---
.../src/Timeseries/transformers.ts | 37 ++++++++----
.../plugin-chart-echarts/src/utils/forecast.ts | 8 ++-
.../plugin-chart-echarts/src/utils/series.ts | 6 +-
.../test/utils/forecast.test.ts | 24 ++++++++
.../plugin-chart-echarts/test/utils/series.test.ts | 17 ++++++
.../test/utils/transformers.test.ts | 69 +++++++++++++++++++++-
7 files changed, 155 insertions(+), 21 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index a2ad93e9578..01a3fe20d79 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -525,9 +525,10 @@ export default function transformProps(
// bug in Echarts - `stackStrategy: 'all'` doesn't work with nulls, so we cast them to 0
series.push({
...transformedSeries,
- data: (transformedSeries.data as any).map(
- (row: [string | number, number]) => [row[0], row[1] ?? 0],
- ),
+ data: (Array.isArray(transformedSeries.data)
+ ? transformedSeries.data
+ : []
+ ).map((row: [string | number, number]) => [row[0], row[1] ?? 0]),
});
} else {
series.push(transformedSeries);
@@ -1040,10 +1041,10 @@ export default function transformProps(
// For axis tooltips, prefer axisValue/axisValueLabel which contains the full label
// even when the axis label is visually truncated
const xValue: number = richTooltip
- ? (params[0].axisValue ??
- params[0].axisValueLabel ??
- params[0].value[xIndex])
- : params.value[xIndex];
+ ? (params[0]?.axisValue ??
+ params[0]?.axisValueLabel ??
+ params[0]?.value?.[xIndex])
+ : params.value?.[xIndex];
const forecastValue: CallbackDataParams[] = richTooltip
? params
: [params];
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
index 8e4919e63fc..eb2883533f2 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -75,31 +75,38 @@ export const getBaselineSeriesForStream = (
series: [string | number, number][][],
seriesType: EchartsTimeseriesSeriesType,
) => {
- const seriesLength = series[0].length;
+ // Series data can be empty or ragged (unequal lengths, null rows) when a
+ // metric column is missing or a series is all-null; index defensively so a
+ // stream-stacked chart doesn't crash while computing the baseline.
+ const seriesLength = series[0]?.length ?? 0;
const baselineSeriesDelta: [string | number, number][] = Array.from(
{ length: seriesLength },
() => [0, 0],
);
- const getVal = (value: number | null) => value ?? 0;
+ const getVal = (value: number | null | undefined) => value ?? 0;
for (let i = 0; i < seriesLength; i += 1) {
let seriesSum = 0;
let weightedSeriesSum = 0;
for (let j = 0; j < series.length; j += 1) {
const delta =
i > 0
- ? getVal(series[j][i][1]) - getVal(series[j][i - 1][1])
- : getVal(series[j][i][1]);
+ ? getVal(series[j]?.[i]?.[1]) - getVal(series[j]?.[i - 1]?.[1])
+ : getVal(series[j]?.[i]?.[1]);
let deltaPrev = 0;
for (let k = 1; k < j - 1; k += 1) {
deltaPrev +=
i > 0
- ? getVal(series[k][i][1]) - getVal(series[k][i - 1][1])
- : getVal(series[k][i][1]);
+ ? getVal(series[k]?.[i]?.[1]) - getVal(series[k]?.[i - 1]?.[1])
+ : getVal(series[k]?.[i]?.[1]);
}
- weightedSeriesSum += (0.5 * delta + deltaPrev) * getVal(series[j][i][1]);
- seriesSum += getVal(series[j][i][1]);
+ weightedSeriesSum +=
+ (0.5 * delta + deltaPrev) * getVal(series[j]?.[i]?.[1]);
+ seriesSum += getVal(series[j]?.[i]?.[1]);
}
- baselineSeriesDelta[i] = [series[0][i][0], -weightedSeriesSum / seriesSum];
+ baselineSeriesDelta[i] = [
+ series[0][i][0],
+ seriesSum === 0 ? 0 : -weightedSeriesSum / seriesSum,
+ ];
}
const baselineSeries = baselineSeriesDelta.reduce<
[string | number, number][]
@@ -430,6 +437,11 @@ export function transformSeries(
return '';
}
const { value, dataIndex, seriesIndex, seriesName } = params;
+ // A rendered point can be a bare null/undefined (empty or all-null
+ // series) rather than a [x, y] tuple; bail out instead of indexing it.
+ if (!Array.isArray(value)) {
+ return '';
+ }
const numericValue = isHorizontal ? value[0] : value[1];
const isSelectedLegend = !legendState || legendState[seriesName];
const isAreaExpand = stack === StackControlsValue.Expand;
@@ -687,7 +699,12 @@ export function transformTimeseriesAnnotation(
const { hideLine, name, opacity, showMarkers, style, width, color } = layer;
const result = annotationData[name];
const isHorizontal = orientation === OrientationType.Horizontal;
- const { records } = result;
+ // `result` can be undefined when a Timeseries annotation layer is shown but
+ // its backing data is missing (source deleted/renamed, filtered to empty, or
+ // the annotation query returned nothing). Guard against it rather than
+ // destructuring directly, mirroring `extractRecordAnnotations`, so the whole
+ // chart doesn't fail to render. (regression from #34709)
+ const records = result?.records;
if (records) {
const data = records.map(record => {
const keys = Object.keys(record);
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
index 18ee19b0d58..f7e7a8f0683 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
@@ -62,7 +62,13 @@ export const extractForecastValuesFromTooltipParams = (
params.forEach(param => {
const { marker, seriesId, value, color } = param;
const context = extractForecastSeriesContext(seriesId);
- const numericValue = isHorizontal ? value[0] : value[1];
+ // `value` may be a bare null/undefined for empty or all-null series;
+ // guard before indexing so the tooltip doesn't throw.
+ const numericValue = Array.isArray(value)
+ ? isHorizontal
+ ? value[0]
+ : value[1]
+ : undefined;
if (typeof numericValue === 'number') {
if (!(context.name in values))
values[context.name] = {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index d8c899be77a..e7ae69595e0 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -1053,13 +1053,15 @@ export function extractTooltipKeys(
if (richTooltip && tooltipSortByMetric) {
return forecastValue
.slice()
- .sort((a, b) => b.data[yIndex] - a.data[yIndex])
+ .sort((a, b) => (b.data?.[yIndex] ?? 0) - (a.data?.[yIndex] ?? 0))
.map(value => value[TOOLTIP_SERIES_KEY]);
}
if (richTooltip) {
return forecastValue.map(s => s[TOOLTIP_SERIES_KEY]);
}
- return [forecastValue[0][TOOLTIP_SERIES_KEY]];
+ // Non-rich (item) tooltips can fire with an empty params array when the
+ // hovered point has no series data; avoid indexing into an empty array.
+ return forecastValue.length ? [forecastValue[0][TOOLTIP_SERIES_KEY]] : [];
}
export function groupData(data: DataRecord[], by?: string | null) {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
index 40a4fed4e76..40a0d2c6028 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
@@ -267,6 +267,30 @@ test('extractForecastValuesFromTooltipParams should extract valid values', () =>
});
});
+// Regression (H3/sc-114915): all-null or empty series can hand the tooltip a
+// bare null/undefined `value` instead of a [x, y] tuple. Indexing it threw
+// "Cannot read properties of null (reading '1')" and killed the render.
+test('extractForecastValuesFromTooltipParams tolerates null/undefined values', () => {
+ expect(() =>
+ extractForecastValuesFromTooltipParams([
+ { marker: '<img>', seriesId: 'nullish', value: null },
+ { marker: '<img>', seriesId: 'undef', value: undefined },
+ { marker: '<img>', seriesId: 'ok', value: [0, 10] },
+ ]),
+ ).not.toThrow();
+ expect(
+ extractForecastValuesFromTooltipParams([
+ { marker: '<img>', seriesId: 'nullish', value: null },
+ { marker: '<img>', seriesId: 'ok', value: [0, 10] },
+ ]),
+ ).toEqual({
+ ok: {
+ marker: '<img>',
+ observation: 10,
+ },
+ });
+});
+
const formatter = getNumberFormatter(NumberFormats.INTEGER);
test('formatForecastTooltipSeries should apply format to value', () => {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index d0ae93f80c0..da9a4b7e400 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -1646,3 +1646,20 @@ test('extractTooltipKeys with non-rich tooltip', () => {
const result = extractTooltipKeys(forecastValue, 1, false, false);
expect(result).toEqual(['foo']);
});
+
+// Regression (H3/sc-114915): an item tooltip firing with no series data passed
+// an empty params array, and `forecastValue[0][TOOLTIP_SERIES_KEY]` threw
+// "Cannot read properties of undefined". It should return an empty list.
+test('extractTooltipKeys with non-rich tooltip and empty params', () => {
+ expect(() => extractTooltipKeys([], 1, false, false)).not.toThrow();
+ expect(extractTooltipKeys([], 1, false, false)).toEqual([]);
+});
+
+// Sorting must tolerate series whose `data` is missing (all-null series).
+test('extractTooltipKeys sorts by metric even when some data is missing', () => {
+ const withMissing = [
+ { seriesId: 'foo' },
+ { data: [0, 2], seriesId: 'bar' },
+ ] as any[];
+ expect(() => extractTooltipKeys(withMissing, 1, true, true)).not.toThrow();
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts
index ef372b65047..a2bf65a3cf8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts
@@ -31,8 +31,9 @@ import {
TimeseriesDataRecord,
} from '@superset-ui/core';
import { supersetTheme } from '@apache-superset/core/theme';
-import { OrientationType } from '../../src';
+import { EchartsTimeseriesSeriesType, OrientationType } from '../../src';
import {
+ getBaselineSeriesForStream,
transformEventAnnotation,
transformFormulaAnnotation,
transformIntervalAnnotation,
@@ -422,4 +423,70 @@ describe('transformTimeseriesAnnotation', () => {
],
]);
});
+
+ // Regression: #34709 replaced the `Array.isArray(result)` guard with an
+ // unguarded `const { records } = result`, so a shown Timeseries annotation
+ // layer whose backing data is missing crashed the entire chart render with
+ // "Cannot read properties of undefined (reading 'records')" (sc-114915).
+ test('does not throw and returns no series when annotation data is missing', () => {
+ let series;
+ expect(() => {
+ series = transformTimeseriesAnnotation(
+ mockTimeseriesAnnotationLayer,
+ 1,
+ mockData,
+ {}, // annotationData has no entry for this shown layer
+ CategoricalColorNamespace.getScale(''),
+ );
+ }).not.toThrow();
+ expect(series).toEqual([]);
+ });
+
+ test('does not throw when annotation result exists but has no records', () => {
+ let series;
+ expect(() => {
+ series = transformTimeseriesAnnotation(
+ mockTimeseriesAnnotationLayer,
+ 1,
+ mockData,
+ { 'Timeseries annotation layer': {} } as AnnotationData,
+ CategoricalColorNamespace.getScale(''),
+ );
+ }).not.toThrow();
+ expect(series).toEqual([]);
+ });
+});
+
+describe('getBaselineSeriesForStream', () => {
+ // Regression (H4/sc-114915): ragged or empty series data drove numeric
+ // indexing into a null/undefined array (e.g. `series[j][i][1]`), throwing
+ // "Cannot read properties of null (reading '<n>')" during stream-stack render.
+ test('does not throw when series data is empty', () => {
+ expect(() =>
+ getBaselineSeriesForStream([], EchartsTimeseriesSeriesType.Line),
+ ).not.toThrow();
+ });
+
+ test('does not throw when series have ragged/missing rows', () => {
+ const ragged = [
+ [
+ [0, 1],
+ [1, 2],
+ [2, 3],
+ ],
+ [[0, 5]], // shorter series: missing later rows
+ ] as [string | number, number][][];
+ let baseline;
+ expect(() => {
+ baseline = getBaselineSeriesForStream(
+ ragged,
+ EchartsTimeseriesSeriesType.Line,
+ );
+ }).not.toThrow();
+ expect((baseline as any).data).toHaveLength(3);
+ // No NaN leaks into the baseline coordinates fed to ECharts.
+ (baseline as any).data.forEach((point: [string | number, number]) => {
+ expect(Number.isNaN(point[1])).toBe(false);
+ });
+ });
});