Skip to content

Commit

Permalink
[bugfix] prevent d3-format from raising (apache#6386)
Browse files Browse the repository at this point in the history
Since apache#6287 and
effectively moving to a new version of d3, d3-format and d3-time-format
raises when receiving invalid input strings.

This code wraps the potential issues inside `try` blocks that will
effectively return an `ERROR` string as output to the formatting
function.
  • Loading branch information
mistercrunch authored and bipinsoniguavus committed Dec 24, 2018
1 parent 454d98c commit 846dfdc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
4 changes: 3 additions & 1 deletion superset/assets/spec/javascripts/modules/utils_spec.jsx
Expand Up @@ -29,6 +29,7 @@ describe('utils', () => {
expect(d3format('.3s', 1234)).toBe('1.23k');
expect(d3format('.3s', 1237)).toBe('1.24k');
expect(d3format('', 1237)).toBe('1.24k');
expect(d3format('.2efd2.ef.2.e', 1237)).toBe('ERROR');
});
});

Expand All @@ -45,8 +46,9 @@ describe('utils', () => {
it('is a function', () => {
expect(typeof d3TimeFormatPreset).toBe('function');
});
it('returns a working time formatter', () => {
it('returns a working formatter', () => {
expect(d3FormatPreset('smart_date')(0)).toBe('1970');
expect(d3FormatPreset('%%GIBBERISH')(0)).toBe('ERROR');
});
});

Expand Down
19 changes: 16 additions & 3 deletions superset/assets/src/modules/utils.js
Expand Up @@ -6,6 +6,7 @@ import { timeFormat as d3TimeFormat } from 'd3-time-format';
import { formatDate, UTC } from './dates';

const siFormatter = d3Format('.3s');
const ERROR_STRING = 'ERROR';

export function defaultNumberFormatter(n) {
let si = siFormatter(n);
Expand All @@ -22,7 +23,13 @@ export function d3FormatPreset(format) {
return formatDate;
}
if (format) {
return d3Format(format);
try {
return d3Format(format);
} catch (e) {
// eslint-disable-next-line no-console
console.warn(e);
return () => ERROR_STRING;
}
}
return defaultNumberFormatter;
}
Expand All @@ -45,12 +52,18 @@ export function d3format(format, number) {
format = format || '.3s';
// Formats a number and memoizes formatters to be reused
if (!(format in formatters)) {
formatters[format] = d3Format(format);
try {
formatters[format] = d3Format(format);
} catch (e) {
// eslint-disable-next-line no-console
console.warn(e);
return ERROR_STRING;
}
}
try {
return formatters[format](number);
} catch (e) {
return 'ERR';
return ERROR_STRING;
}
}

Expand Down
4 changes: 3 additions & 1 deletion superset/assets/src/visualizations/nvd3/NVD3Vis.js
Expand Up @@ -456,11 +456,13 @@ function nvd3Vis(element, props) {
chart.xScale(d3.scale.log());
}

let xAxisFormatter = d3FormatPreset(xAxisFormat);
let xAxisFormatter;
if (isTimeSeries) {
xAxisFormatter = d3TimeFormatPreset(xAxisFormat);
// In tooltips, always use the verbose time format
chart.interactiveLayer.tooltip.headerFormatter(formatDateVerbose);
} else {
xAxisFormatter = d3FormatPreset(xAxisFormat);
}
if (chart.x2Axis && chart.x2Axis.tickFormat) {
chart.x2Axis.tickFormat(xAxisFormatter);
Expand Down

0 comments on commit 846dfdc

Please sign in to comment.