Prevent DAG crashes because of empty service name string (jaegertraci…

…ng#656) * Prevent DAG crashes because of empty service name string Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com> * Adding tests for empty service names Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
vvvprabhakar · Jul 4, 2021 · 5a2940a · 5a2940a
1 parent 24f848e
commit 5a2940a
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 15 deletions.
diff --git a/packages/jaeger-ui/src/components/DependencyGraph/DAG.js b/packages/jaeger-ui/src/components/DependencyGraph/DAG.js
@@ -35,26 +35,34 @@ export default class DAG extends React.Component {
     serviceCalls: [],
   };
 
+  constructor(props) {
+    super(props);
+    this.cytoscapeRef = React.createRef();
+  }
+
   componentDidMount() {
     const { serviceCalls } = this.props;
     const nodeMap = {};
     const nodes = [];
     const edges = [];
     serviceCalls.forEach(d => {
-      if (!nodeMap[d.parent]) {
-        nodes.push({ data: { id: d.parent } });
-        nodeMap[d.parent] = true;
+      if (d.parent.trim().length !== 0 && d.child.trim().length !== 0) {
+        if (!nodeMap[d.parent]) {
+          nodes.push({ data: { id: d.parent } });
+          nodeMap[d.parent] = true;
+        }
+        if (!nodeMap[d.child]) {
+          nodes.push({ data: { id: d.child } });
+          nodeMap[d.child] = true;
+        }
+        edges.push({
+          data: { source: d.parent, target: d.child, label: `${d.callCount}` },
+        });
       }
-      if (!nodeMap[d.child]) {
-        nodes.push({ data: { id: d.child } });
-        nodeMap[d.child] = true;
-      }
-      edges.push({
-        data: { source: d.parent, target: d.child, label: `${d.callCount}` },
-      });
     });
+
     cytoscape({
-      container: document.getElementById('cy'),
+      container: this.cytoscapeRef.current,
       boxSelectionEnabled: false,
       autounselectify: true,
       layout: {
@@ -102,6 +110,7 @@ export default class DAG extends React.Component {
           left: 0,
           top: 0,
         }}
+        ref={this.cytoscapeRef}
       />
     );
   }

diff --git a/packages/jaeger-ui/src/components/DependencyGraph/DAG.test.js b/packages/jaeger-ui/src/components/DependencyGraph/DAG.test.js
@@ -12,14 +12,48 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-/* eslint-disable import/first */
-jest.mock('cytoscape');
-
 import React from 'react';
 import { mount } from 'enzyme';
-
 import DAG from './DAG';
 
+// mock canvas API (we don't care about canvas results)
+
+window.HTMLCanvasElement.prototype.getContext = function() {
+  return {
+    fillRect() {},
+    clearRect() {},
+    getImageData(x, y, w, h) {
+      return {
+        data: new Array(w * h * 4),
+      };
+    },
+    putImageData() {},
+    createImageData() {
+      return [];
+    },
+    setTransform() {},
+    drawImage() {},
+    save() {},
+    fillText() {},
+    restore() {},
+    beginPath() {},
+    moveTo() {},
+    lineTo() {},
+    closePath() {},
+    stroke() {},
+    translate() {},
+    scale() {},
+    rotate() {},
+    arc() {},
+    fill() {},
+    measureText() {
+      return { width: 0 };
+    },
+    transform() {},
+    rect() {},
+    clip() {},
+  };
+};
 describe('<DAG>', () => {
   it('does not explode', () => {
     const serviceCalls = [
@@ -31,4 +65,15 @@ describe('<DAG>', () => {
     ];
     expect(mount(<DAG serviceCalls={serviceCalls} />)).toBeDefined();
   });
+
+  it('does not explode with empty strings or string with only spaces', () => {
+    const serviceCalls = [
+      {
+        callCount: 1,
+        child: '',
+        parent: ' ',
+      },
+    ];
+    expect(mount(<DAG serviceCalls={serviceCalls} />)).toBeDefined();
+  });
 });