Skip to content

Commit

Permalink
fix(client): fix error handling
Browse files Browse the repository at this point in the history
Related to #866
  • Loading branch information
philippfromme authored and nikku committed Dec 1, 2018
1 parent b0f4db0 commit fe101b9
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 46 deletions.
4 changes: 1 addition & 3 deletions client/src/app/__tests__/AppSpec.js
Expand Up @@ -651,9 +651,7 @@ describe('<App>', function() {

const file1 = createFile('1.bpmn');

openedTabs = [
...(await app.openFiles([ file1 ])),
];
openedTabs = await app.openFiles([ file1 ]);

// assume
const {
Expand Down
8 changes: 6 additions & 2 deletions client/src/app/tabs/MultiSheetTab.js
Expand Up @@ -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,
Expand Down
36 changes: 24 additions & 12 deletions client/src/app/tabs/bpmn/BpmnEditor.js
Expand Up @@ -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) => {
Expand Down Expand Up @@ -247,7 +250,9 @@ export class BpmnEditor extends CachedComponent {
modeler
} = this.getCached();

const xml = this.props.xml;
const {
xml
} = this.props;

if (xml !== modeler.lastXML) {

Expand All @@ -257,11 +262,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({
Expand All @@ -284,7 +290,9 @@ export class BpmnEditor extends CachedComponent {
modeler.lastXML = xml;

if (err) {
this.props.onError(err);
this.handleError({
error: err
});

return reject(err);
}
Expand All @@ -305,16 +313,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);
}
Expand Down
56 changes: 49 additions & 7 deletions client/src/app/tabs/bpmn/__tests__/BpmnEditorSpec.js
Expand Up @@ -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;

Expand Down Expand Up @@ -116,8 +116,10 @@ describe.only('<BpmnEditor>', function() {
bpmnEditor,
wrapper
} = renderBpmnEditor(diagramXML, {
propertiesPanel: {
open: false
layout: {
propertiesPanel: {
open: false
}
}
});

Expand Down Expand Up @@ -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
Expand Down
30 changes: 21 additions & 9 deletions client/src/app/tabs/cmmn/CmmnEditor.js
Expand Up @@ -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) => {
Expand Down Expand Up @@ -181,15 +183,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
});
}
});
}
Expand All @@ -208,7 +214,9 @@ export class CmmnEditor extends CachedComponent {
modeler.lastXML = xml;

if (err) {
this.props.onError(err);
this.handleError({
error: err
});

return reject(err);
}
Expand All @@ -229,16 +237,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);
}
Expand Down
28 changes: 15 additions & 13 deletions client/src/app/tabs/dmn/DmnEditor.js
Expand Up @@ -152,7 +152,7 @@ class DmnEditor extends CachedComponent {

const {
error,
warmings
warnings
} = event;

const {
Expand All @@ -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);
Expand Down Expand Up @@ -309,9 +305,11 @@ class DmnEditor extends CachedComponent {
error
} = event;

console.warn('modeling error', error);
const {
onError
} = this.props;

this.props.onError(error);
onError(error);
}

handleMinimapToggle = (event) => {
Expand Down Expand Up @@ -421,7 +419,9 @@ class DmnEditor extends CachedComponent {
modeler.lastXML = xml;

if (err) {
this.props.onError(err);
this.handleError({
error: err
});

return reject(err);
}
Expand All @@ -444,14 +444,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);
}
Expand Down

0 comments on commit fe101b9

Please sign in to comment.