Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling #4135

Merged
merged 3 commits into from Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/api-reference/deck.md
Expand Up @@ -384,6 +384,17 @@ Callback arguments:
* `gl` - the WebGL context.



##### `onError` (Function, optional)

Called if deck.gl encounters an error. By default, deck logs the error to console and attempt to continue rendering the rest of the scene.

Callback arguments:

* `error` ([Error](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error))
* `source` - the source of the error, likely a Layer instance


##### `_onMetrics` (Function, optional) **Experimental**

Called once every second with performance metrics.
Expand Down
6 changes: 6 additions & 0 deletions modules/core/src/lib/deck-picker.js
Expand Up @@ -44,12 +44,17 @@ export default class DeckPicker {
layerId: null,
info: null
};
this._onError = null;
}

setProps(props) {
if ('layerFilter' in props) {
this.layerFilter = props.layerFilter;
}

if ('onError' in props) {
this._onError = props.onError;
}
}

finalize() {
Expand Down Expand Up @@ -316,6 +321,7 @@ export default class DeckPicker {
this.pickLayersPass.render({
layers,
layerFilter: this.layerFilter,
onError: this._onError,
viewports,
onViewportActive,
pickingFBO,
Expand Down
6 changes: 6 additions & 0 deletions modules/core/src/lib/deck-renderer.js
Expand Up @@ -16,6 +16,7 @@ export default class DeckRenderer {
this._needsRedraw = 'Initial render';
this.renderBuffers = [];
this.lastPostProcessEffect = null;
this._onError = null;
}

setProps(props) {
Expand All @@ -28,6 +29,10 @@ export default class DeckRenderer {
this.drawPickingColors = props.drawPickingColors;
this._needsRedraw = 'drawPickingColors changed';
}

if ('onError' in props) {
this._onError = props.onError;
}
}

/*
Expand All @@ -46,6 +51,7 @@ export default class DeckRenderer {
const layerPass = this.drawPickingColors ? this.pickLayersPass : this.drawLayersPass;

opts.layerFilter = this.layerFilter;
opts.onError = this._onError;
opts.effects = opts.effects || [];
opts.target = opts.target || Framebuffer.getDefaultFramebuffer(this.gl);

Expand Down
2 changes: 2 additions & 0 deletions modules/core/src/lib/deck.js
Expand Up @@ -81,6 +81,7 @@ function getPropTypes(PropTypes) {
onBeforeRender: PropTypes.func,
onAfterRender: PropTypes.func,
onLoad: PropTypes.func,
onError: PropTypes.func,

// Debug settings
debug: PropTypes.bool,
Expand Down Expand Up @@ -117,6 +118,7 @@ const defaultProps = {
onBeforeRender: noop,
onAfterRender: noop,
onLoad: noop,
onError: null,
_onMetrics: null,

getCursor,
Expand Down
58 changes: 23 additions & 35 deletions modules/core/src/lib/layer-manager.js
Expand Up @@ -83,6 +83,7 @@ export default class LayerManager {
this._needsRedraw = 'Initial render';
this._needsUpdate = false;
this._debug = false;
this._onError = null;

this.activateViewport = this.activateViewport.bind(this);

Expand Down Expand Up @@ -154,6 +155,10 @@ export default class LayerManager {
if ('layers' in props) {
this.setLayers(props.layers);
}

if ('onError' in props) {
this._onError = props.onError;
}
}

// Supply a new layer list, initiating sublayer generation and layer matching
Expand Down Expand Up @@ -220,6 +225,14 @@ export default class LayerManager {
return this;
}

_handleError(stage, error, layer) {
if (this._onError) {
this._onError(error, layer);
} else {
log.error(`error during ${stage} of ${layerName(layer)}`, error)();
}
}

// Match all layers, checking for caught errors
// To avoid having an exception in one layer disrupt other layers
// TODO - mark layers with exceptions as bad and remove from rendering cycle?
Expand All @@ -238,10 +251,10 @@ export default class LayerManager {
const generatedLayers = [];

// Match sublayers
const error = this._updateSublayersRecursively(newLayers, oldLayerMap, generatedLayers);
this._updateSublayersRecursively(newLayers, oldLayerMap, generatedLayers);

// Finalize unmatched layers
const error2 = this._finalizeOldLayers(oldLayerMap);
this._finalizeOldLayers(oldLayerMap);

let needsUpdate = false;
for (const layer of generatedLayers) {
Expand All @@ -253,19 +266,11 @@ export default class LayerManager {

this._needsUpdate = needsUpdate;
this.layers = generatedLayers;

// Throw first error found, if any
const firstError = error || error2;
if (firstError) {
throw firstError;
}
}

/* eslint-disable complexity,max-statements */
// Note: adds generated layers to `generatedLayers` array parameter
_updateSublayersRecursively(newLayers, oldLayerMap, generatedLayers) {
let error = null;

for (const newLayer of newLayers) {
newLayer.context = this.context;

Expand All @@ -287,44 +292,35 @@ export default class LayerManager {
}

if (!oldLayer) {
const err = this._initializeLayer(newLayer);
error = error || err;
this._initializeLayer(newLayer);
} else {
this._transferLayerState(oldLayer, newLayer);
const err = this._updateLayer(newLayer);
error = error || err;
this._updateLayer(newLayer);
}
generatedLayers.push(newLayer);

// Call layer lifecycle method: render sublayers
sublayers = newLayer.isComposite && newLayer.getSubLayers();
// End layer lifecycle method: render sublayers
} catch (err) {
log.warn(`error during matching of ${layerName(newLayer)}`, err)();
error = error || err; // Record first exception
this._handleError('matching', err, newLayer); // Record first exception
}

if (sublayers) {
const err = this._updateSublayersRecursively(sublayers, oldLayerMap, generatedLayers);
error = error || err;
this._updateSublayersRecursively(sublayers, oldLayerMap, generatedLayers);
}
}

return error;
}
/* eslint-enable complexity,max-statements */

// Finalize any old layers that were not matched
_finalizeOldLayers(oldLayerMap) {
let error = null;
for (const layerId in oldLayerMap) {
const layer = oldLayerMap[layerId];
if (layer) {
const err = this._finalizeLayer(layer);
error = error || err;
this._finalizeLayer(layer);
}
}
return error;
}

// EXCEPTION SAFE LAYER ACCESS
Expand All @@ -335,12 +331,9 @@ export default class LayerManager {
layer._initialize();
layer.lifecycle = LIFECYCLE.INITIALIZED;
} catch (err) {
log.warn(`error while initializing ${layerName(layer)}\n`, err)();
return err;
this._handleError('initialization', err, layer);
// TODO - what should the lifecycle state be here? LIFECYCLE.INITIALIZATION_FAILED?
}

return null;
}

_transferLayerState(oldLayer, newLayer) {
Expand All @@ -357,11 +350,8 @@ export default class LayerManager {
try {
layer._update();
} catch (err) {
log.warn(`error during update of ${layerName(layer)}`, err)();
// Save first error
return err;
this._handleError('update', err, layer);
}
return null;
}

// Finalizes a single layer
Expand All @@ -374,9 +364,7 @@ export default class LayerManager {
layer._finalize();
layer.lifecycle = LIFECYCLE.FINALIZED;
} catch (err) {
log.warn(`error during finalization of ${layerName(layer)}`, err)();
return err;
this._handleError('finalization', err, layer);
}
return null;
}
}
3 changes: 2 additions & 1 deletion modules/core/src/lib/layer.js
Expand Up @@ -616,7 +616,8 @@ export default class Layer extends Component {

const currentProps = this.props;
// Overwrite this.props during redraw to use in-transition prop values
this.props = this.internalState.propsInTransition;
// `internalState.propsInTransition` could be missing if `updateState` failed
this.props = this.internalState.propsInTransition || currentProps;

const {opacity} = this.props;
// apply gamma to opacity to make it visually "linear"
Expand Down
21 changes: 15 additions & 6 deletions modules/core/src/passes/layers-pass.js
@@ -1,6 +1,7 @@
import GL from '@luma.gl/constants';
import Pass from './pass';
import {clear, setParameters, withParameters, cssToDeviceRatio} from '@luma.gl/core';
import log from '../utils/log';

export default class LayersPass extends Pass {
render(props) {
Expand Down Expand Up @@ -45,7 +46,7 @@ export default class LayersPass extends Pass {
// intersect with the picking rect
_drawLayersInViewport(
gl,
{layers, layerFilter, viewport, view, pass = 'unknown', effects, moduleParameters}
{layers, layerFilter, onError, viewport, view, pass = 'unknown', effects, moduleParameters}
) {
const glViewport = getGLViewport(gl, {viewport});

Expand Down Expand Up @@ -92,11 +93,19 @@ export default class LayersPass extends Pass {
const uniforms = Object.assign({}, layer.context.uniforms, {layerIndex});
const layerParameters = this.getLayerParameters(layer, layerIndex);

layer.drawLayer({
moduleParameters: _moduleParameters,
uniforms,
parameters: layerParameters
});
try {
layer.drawLayer({
moduleParameters: _moduleParameters,
uniforms,
parameters: layerParameters
});
} catch (err) {
if (onError) {
onError(err, layer);
} else {
log.error(`error during drawing of ${layer}`, err)();
}
}
}
});

Expand Down
41 changes: 40 additions & 1 deletion test/modules/core/lib/layer-manager.spec.js
Expand Up @@ -19,7 +19,7 @@
// THE SOFTWARE.
/* eslint-disable func-style, no-console, max-len */
import test from 'tape-catch';
import {LayerManager, Layer, CompositeLayer} from 'deck.gl';
import {LayerManager, ScatterplotLayer, Layer, CompositeLayer} from 'deck.gl';
import {gl} from '@deck.gl/test-utils';

class TestLayer extends Layer {
Expand Down Expand Up @@ -174,3 +174,42 @@ test('LayerManager#setLayers', t => {

t.end();
});

test('LayerManager#error handling', t => {
const errorArgs = [];
const onError = (error, layer) => errorArgs.push({error, layer});

class BadLayer extends Layer {
initializeState() {}

updateState() {
if (this.props.throw) {
throw new Error();
}
}
}

const layerManager = new LayerManager(gl);
layerManager.setProps({onError});

layerManager.setLayers([
new ScatterplotLayer({id: 'scatterplot'}),
new BadLayer({id: 'crash-on-init', throw: true}),
new BadLayer({id: 'crash-on-update', throw: false})
]);

t.is(errorArgs.length, 1, 'onError is called');
t.is(errorArgs[0].layer.id, 'crash-on-init', 'onError is called with correct args');

layerManager.setLayers([
new ScatterplotLayer({id: 'scatterplot'}),
new BadLayer({id: 'crash-on-init', throw: true}),
new BadLayer({id: 'crash-on-update', throw: true})
]);

t.is(errorArgs.length, 3, 'onError is called');
t.is(errorArgs[1].layer.id, 'crash-on-init', 'onError is called with correct args');
t.is(errorArgs[2].layer.id, 'crash-on-update', 'onError is called with correct args');

t.end();
});