fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on … #898

lefex · 2022-03-09T00:27:55Z

…'CanvasRenderingContext2D': The provided double value is non-finite.

at createRadialGradient. Excute npm run test error fix! (#16649)

The bug in Echarts apache/echarts#16649

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

pissang · 2022-03-10T02:38:04Z

src/canvas/helper.ts

@@ -4,30 +4,36 @@ import { GradientObject } from '../graphic/Gradient';
 import { RectLike } from '../core/BoundingRect';
 import Path from '../graphic/Path';

+export function safeNum(num: number, defalutNum: number = 0, negative = false) {
+    // null NaN or empty string
+    if (num === null || isNaN(num) || `${num}` === '') {


Seems isNaN(num) also includes null and '' check.

the last submit have removed

pissang · 2022-03-10T02:38:46Z

src/canvas/helper.ts

@@ -4,30 +4,41 @@ import { GradientObject } from '../graphic/Gradient';
 import { RectLike } from '../core/BoundingRect';
 import Path from '../graphic/Path';

+export function safeNum(num: number, defalutNum: number = 0, negative = false) {


Use name nonNegative will be better.

have changed

plainheart · 2022-03-10T05:26:21Z

If we filter these illegal values in the underlying zrender without any warnings, will it hide potential errors of arguments passed from the upper application like ECharts? Most of the time, I think such an issue is caused by error arguments from the callers.

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

lefex · 2022-03-10T07:19:46Z

If we filter these illegal values in the underlying zrender without any warnings, will it hide potential errors of arguments passed from the upper application like ECharts? Most of the time, I think such an issue is caused by error arguments from the callers.

I think we should check the parameters to make sure there are no errors.

plainheart · 2022-03-10T07:33:28Z

It may be more difficult for the developers to find out why they didn't get the expected result as no errors were thrown and no warnings were output.

pissang · 2022-03-10T07:41:45Z

Usually we prefer warning logs over exceptions when having invalid values.

Ovilia · 2022-03-11T07:58:53Z

How about giving warning as well as doing such safe guarding?
You can use warn function in util/log to do so.

lefex · 2022-03-11T08:08:06Z

How about giving warning as well as doing such safe guarding? You can use warn function in util/log to do so.

ok！

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

lefex · 2022-03-11T14:29:16Z

I add a warning in this pr . I'm not going to change the createLinearGradient method.

@pissang @Ovilia

pissang · 2022-03-18T02:05:24Z

src/canvas/helper.ts

+        return false;
+    }
+    // NaN, Infinity, undefined, 'xx'
+    if (!isFinite(num)) {


!isFinite also includes num === null check.

These three check can be merged to:

if (!isFinite(num) || (nonNegative && num < 0 { ... }

Sorry it's my mistake. null can be convert to 0 here. I rechecked if null can be used as argument of createLinearGradient and seems it can. But undefined will throw an non-finite exception. So if we mainly wan using this check to avoid exceptions, we can still remove null check.

pissang · 2022-03-18T02:22:39Z

src/canvas/helper.ts

    if (!obj.global) {
        x = x * width + rect.x;
        y = y * height + rect.y;
        r = r * min;
    }

+    if (!isSafeNum(x) || !isSafeNum(y) || !isSafeNum(r, true)) {
+        console.warn('The provided value is non-finite. You must provide x and y.');
+        return;


return nothing here is not safe. Because we expect this method to return a CanvasGradient and set it to CanvasRenderingContext. If we return null, it will be an undefined behavior.

I think we can fallback to default value when the x, y, r is invalid like createLinearGradient did.

Also there are several concerns about this warn message being to frequently:

It should be warn only once if the CanvasGradientObject is not changed. Use WeakMap to mark this CanvasGradientObject has been warned may be a solution.

The boundingRect sometimes may be infinite or NaN(when we don't want to show the element) and leading this x, y, r to be invalid. In this case, the error message will not be helpful but confuse the
developers. So it's better to check the obj.x, obj.y, obj.r earlier and give warning message. Then we do check x, y, r again and fallback to default value.

The check of obj.x, obj.y, obj.r and warning message can be done only when process.env.NODE_ENV !== 'production' so these can be ommitted in the production environment.

At last the createLinearGradient should do the same.

The obj is changed when mouse hover, can not use WeakMap to add warning log only once. So I added a guard to ensure that the parameters are valid.

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

pissang · 2022-04-15T04:02:49Z

src/canvas/helper.ts

    if (!obj.global) {
        x = x * width + rect.x;
        y = y * height + rect.y;
        r = r * min;
    }

+    x = isSafeNum(x) ? x : 0.5;
+    y = isSafeNum(y) ? y : 0.5;
+    r = isSafeNum(r, true) ? r : 0.5;


Seems only one number needs to be check positive. In this case I will prefer keeping the isSafeNum simpler and more cohesion that only accept one parameter number. Monomorphic function is always better than polymorphic function, on both code style and performance.

So the r check can be

r = isSafeNum(r) && r >= 0 ? r : 0.5

pissang · 2022-04-15T04:04:35Z

test/ut/spec/contain/Sector.test.ts

@@ -12,7 +12,7 @@ describe('Path', function () {
                cy: 625.5,
                startAngle: -1.5707963267948966,
                endAngle: 4.71238898038469,
-                innerCornerRadius: 5,


This change seems to be unnecessary?

fix(gradient): #898 has bug about safe checkingand breaks all gradients

lefex added 4 commits March 9, 2022 08:20

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

f32ae95

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

6248d21

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

3194c6a

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

91f2bdd

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

pissang reviewed Mar 10, 2022

View reviewed changes

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

8bcda4e

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

7a4e426

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

pissang reviewed Mar 18, 2022

View reviewed changes

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on …

6c4875c

…'CanvasRenderingContext2D': The provided double value is non-finite. at createRadialGradient. Excute npm run test error fix! (#16649)

pissang reviewed Apr 15, 2022

View reviewed changes

fix: Uncaught TypeError: Failed to execute 'createRadialGradient'

6e994ef

pissang approved these changes Apr 15, 2022

View reviewed changes

pissang merged commit ddc7e8b into ecomfe:master Apr 15, 2022

Ovilia added a commit that referenced this pull request Jun 7, 2022

fix(gradient): #898 has bug about safe checkingand breaks all gradients

d9e9187

Ovilia mentioned this pull request Jun 7, 2022

fix(gradient): #898 has bug about safe checkingand breaks all gradients #919

Merged

Ovilia added a commit that referenced this pull request Jun 7, 2022

Merge pull request #919 from ecomfe/fix-gradient

f1085eb

fix(gradient): #898 has bug about safe checkingand breaks all gradients

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on … #898

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on … #898

lefex commented Mar 9, 2022

pissang Mar 10, 2022

lefex Mar 10, 2022 •

edited

Loading

pissang Mar 10, 2022

lefex Mar 10, 2022

plainheart commented Mar 10, 2022

lefex commented Mar 10, 2022

plainheart commented Mar 10, 2022

pissang commented Mar 10, 2022 •

edited

Loading

Ovilia commented Mar 11, 2022

lefex commented Mar 11, 2022

lefex commented Mar 11, 2022 •

edited

Loading

pissang Mar 18, 2022

lefex Mar 19, 2022

pissang Mar 20, 2022

pissang Mar 18, 2022

lefex Mar 21, 2022

pissang Apr 15, 2022

pissang Apr 15, 2022

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on … #898

fix: Uncaught TypeError: Failed to execute 'createRadialGradient' on … #898

Conversation

lefex commented Mar 9, 2022

Choose a reason for hiding this comment

lefex Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plainheart commented Mar 10, 2022

lefex commented Mar 10, 2022

plainheart commented Mar 10, 2022

pissang commented Mar 10, 2022 • edited Loading

Ovilia commented Mar 11, 2022

lefex commented Mar 11, 2022

lefex commented Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lefex Mar 10, 2022 •

edited

Loading

pissang commented Mar 10, 2022 •

edited

Loading

lefex commented Mar 11, 2022 •

edited

Loading