fix(client): fix error handling

Related to #866
camunda · Sep 17, 2018 · dcc8864 · dcc8864
1 parent ed0a483
commit dcc8864
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 46 deletions.
diff --git a/client/src/app/__tests__/AppSpec.js b/client/src/app/__tests__/AppSpec.js
@@ -651,9 +651,7 @@ describe('<App>', function() {
 
         const file1 = createFile('1.bpmn');
 
-        openedTabs = [
-          ...(await app.openFiles([ file1 ])),
-        ];
+        openedTabs = await app.openFiles([ file1 ]);
 
         // assume
         const {

diff --git a/client/src/app/tabs/MultiSheetTab.js b/client/src/app/tabs/MultiSheetTab.js
@@ -63,15 +63,19 @@ class MultiSheetTab extends CachedComponent {
 
   handleChanged = (newState) => {
 
-    const { tab, xml } = this.props;
+    const {
+      onChanged,
+      tab,
+      xml
+    } = this.props;
 
     const {
       dirty
     } = newState;
 
     const { lastXML } = this.getCached();
 
-    this.props.onChanged(
+    onChanged(
       tab,
       {
         ...newState,

diff --git a/client/src/app/tabs/bpmn/BpmnEditor.js b/client/src/app/tabs/bpmn/BpmnEditor.js
@@ -170,21 +170,24 @@ export class BpmnEditor extends CachedComponent {
       errors
     } = event;
 
-    console.warn('element template errors', errors);
-
     errors.forEach(error => {
-      this.props.onError(error);
+      this.handleError({ error });
     });
   }
 
   handleError = (event) => {
+
+    debugger
+
     const {
       error
     } = event;
 
-    console.warn('modeling error', error);
+    const {
+      onError
+    } = this.props;
 
-    this.props.onError(error);
+    onError(error);
   }
 
   updateState = (event) => {
@@ -246,7 +249,9 @@ export class BpmnEditor extends CachedComponent {
       modeler
     } = this.getCached();
 
-    const xml = this.props.xml;
+    const {
+      xml
+    } = this.props;
 
     if (xml !== modeler.lastXML) {
 
@@ -256,11 +261,12 @@ export class BpmnEditor extends CachedComponent {
         loading: true
       });
 
-      // TODO(nikku): handle errors
       // TODO(nikku): apply default element templates to initial diagram
       modeler.importXML(xml, (err) => {
         if (err) {
-          this.props.onError(err);
+          return this.handleError({
+            error: err
+          });
         }
 
         this.setState({
@@ -283,7 +289,9 @@ export class BpmnEditor extends CachedComponent {
         modeler.lastXML = xml;
 
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
 
           return reject(err);
         }
@@ -304,16 +312,20 @@ export class BpmnEditor extends CachedComponent {
         let contents;
 
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
 
-          reject(err);
+          return reject(err);
         }
 
         if (type !== 'svg') {
           try {
             contents = generateImage(type, svg);
           } catch (err) {
-            this.props.onError(err);
+            this.handleError({
+              error: err
+            });
 
             return reject(err);
           }

diff --git a/client/src/app/tabs/bpmn/__tests__/BpmnEditorSpec.js b/client/src/app/tabs/bpmn/__tests__/BpmnEditorSpec.js
@@ -18,7 +18,7 @@ import { insertCSS } from 'test/helper';
 insertCSS('test.css', '.test-content-container { position: relative; }');
 
 
-describe.only('<BpmnEditor>', function() {
+describe('<BpmnEditor>', function() {
 
   let createCachedSpy;
 
@@ -116,8 +116,10 @@ describe.only('<BpmnEditor>', function() {
         bpmnEditor,
         wrapper
       } = renderBpmnEditor(diagramXML, {
-        propertiesPanel: {
-          open: false
+        layout: {
+          propertiesPanel: {
+            open: false
+          }
         }
       });
 
@@ -150,22 +152,62 @@ describe.only('<BpmnEditor>', function() {
 
   });
 
+
+  describe('errors', function() {
+
+    // TODO
+    it('should handle template error');
+
+
+    // TODO(philippfromme): why does import not return error?
+    it.skip('should handle import error', function() {
+
+      // given
+      const onErrorSpy = sinon.spy();
+
+      const errorXML = 'foo';
+
+      // when
+      renderBpmnEditor(errorXML, {
+        onError: onErrorSpy
+      });
+
+      // then
+      expect(onErrorSpy).to.have.been.called;
+    });
+
+    it('should handle export error', function() {
+      // TODO(philippfromme): how to make #getXML throw?
+    });
+
+
+    it('should handle export error', function() {
+      // TODO(philippfromme): how to make #exportAs throw?
+    });
+
+  });
+
 });
 
 function noop() {}
 
 function renderBpmnEditor(xml, options = {}) {
   const {
-    minimap,
-    propertiesPanel
+    layout,
+    onError,
+    onLayoutChanged
   } = options;
 
+  const minimap = layout && layout.minimap,
+        propertiesPanel = layout && layout.propertiesPanel;
+
   const slotFillRoot = mount(
     <SlotFillRoot>
       <BpmnEditorWithCachedState
         id="foo"
-        xml={ diagramXML }
-        onLayoutChanged={ options.onLayoutChanged || noop }
+        xml={ xml }
+        onLayoutChanged={ onLayoutChanged || noop }
+        onError={ onError || noop }
         layout={ {
           minimap: {
             open: minimap ? minimap.open : true

diff --git a/client/src/app/tabs/cmmn/CmmnEditor.js b/client/src/app/tabs/cmmn/CmmnEditor.js
@@ -125,9 +125,11 @@ export class CmmnEditor extends CachedComponent {
       error
     } = event;
 
-    console.warn('modeling error', error);
+    const {
+      onError
+    } = this.props;
 
-    this.props.onError(error);
+    onError(error);
   }
 
   updateState = (event) => {
@@ -180,15 +182,19 @@ export class CmmnEditor extends CachedComponent {
       modeler
     } = this.getCached();
 
-    const xml = this.props.xml;
+    const {
+      xml
+    } = this.props;
 
     if (xml !== modeler.lastXML) {
 
       modeler.lastXML = xml;
 
-      modeler.importXML(xml, function(err) {
+      modeler.importXML(xml, (err) => {
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
         }
       });
     }
@@ -207,7 +213,9 @@ export class CmmnEditor extends CachedComponent {
         modeler.lastXML = xml;
 
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
 
           return reject(err);
         }
@@ -228,16 +236,20 @@ export class CmmnEditor extends CachedComponent {
         let contents;
 
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
 
-          reject(err);
+          return reject(err);
         }
 
         if (type !== 'svg') {
           try {
             contents = generateImage(type, svg);
           } catch (err) {
-            this.props.onError(err);
+            this.handleError({
+              error: err
+            });
 
             return reject(err);
           }

diff --git a/client/src/app/tabs/dmn/DmnEditor.js b/client/src/app/tabs/dmn/DmnEditor.js
@@ -152,7 +152,7 @@ class DmnEditor extends CachedComponent {
 
     const {
       error,
-      warmings
+      warnings
     } = event;
 
     const {
@@ -164,15 +164,11 @@ class DmnEditor extends CachedComponent {
     } = this.props;
 
     if (error) {
-      console.error('imported with error', error);
-
-      this.props.onError(error);
-
-      return;
+      return this.handleError({ error });
     }
 
-    if (warmings.length) {
-      console.error('imported with warnings', warmings);
+    if (warnings.length) {
+      console.error('imported with warnings', warnings);
     }
 
     const initialView = modeler._getInitialView(modeler._views);
@@ -303,9 +299,11 @@ class DmnEditor extends CachedComponent {
       error
     } = event;
 
-    console.warn('modeling error', error);
+    const {
+      onError
+    } = this.props;
 
-    this.props.onError(error);
+    onError(error);
   }
 
   handleMinimapToggle = (event) => {
@@ -415,7 +413,9 @@ class DmnEditor extends CachedComponent {
         modeler.lastXML = xml;
 
         if (err) {
-          this.props.onError(err);
+          this.handleError({
+            error: err
+          });
 
           return reject(err);
         }
@@ -438,14 +438,16 @@ class DmnEditor extends CachedComponent {
         let contents;
 
         if (err) {
-          reject(err);
+          return reject(err);
         }
 
         if (type !== 'svg') {
           try {
             contents = generateImage(type, svg);
           } catch (err) {
-            this.props.onError(err);
+            this.handleError({
+              error: err
+            });
 
             return reject(err);
           }