Throw exceptions when decorating destroyed marker layers #13817

Merged
merged 1 commit into from Feb 14, 2017

Projects

None yet

1 participant

@nathansobo
Contributor

We're seeing exceptions like the following:

TypeError Cannot read property 'findMarkers' of undefined 
    /app.asar/src/decoration-manager.js:151:20 module.exports.DecorationManager.decorationsStateForScreenRowRange
    /app.asar/src/text-editor.js:1698:37 module.exports.TextEditor.decorationsStateForScreenRowRange
    /app.asar/src/text-editor-presenter.js:1381:44 module.exports.TextEditorPresenter.fetchDecorations
    /app.asar/src/text-editor-presenter.js:125:14 module.exports.TextEditorPresenter.getPreMeasurementState
    /app.asar/src/text-editor-component.js:264:60 module.exports.TextEditorComponent.updateSyncPreMeasurement
    /app.asar/src/text-editor-component.js:196:12 module.exports.TextEditorComponent.updateSync
    /app.asar/src/text-editor-component.js:303:21 module.exports.TextEditorComponent.becameVisible
    /app.asar/src/text-editor-component.js:994:16 module.exports.TextEditorComponent.checkForVisibilityChange
    /app.asar/src/text-editor-component.js:980:17 module.exports.TextEditorComponent.pollDOM
    /app.asar/src/text-editor-component.js:3:59 none
    /app.asar/src/view-registry.js:263:9 module.exports.ViewRegistry.performDocumentPoll
    /app.asar/src/view-registry.js:226:14 module.exports.ViewRegistry.performDocumentUpdate
    /app.asar/src/view-registry.js:3:59 none

One way these could occur is if a package attempts to decorate an already destroyed marker layer. If that happened, we would previously silently allow the decoration to be created and then end up querying the editor's display layer for the destroyed layer's id, getting an undefined result.

/cc @maxbrunsfeld

@nathansobo nathansobo Throw exceptions when decorating destroyed marker layers
4abcace
@nathansobo
Contributor

Failures are spurious and we need to look into them, but I'm going to merge.

@nathansobo nathansobo merged commit edc9745 into master Feb 14, 2017

3 of 5 checks passed

ci/circleci Your tests failed on CircleCI
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@nathansobo nathansobo deleted the ns-dont-decorate-destroyed-layers branch Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment