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

removeFeatureState crashes map if feature state isn't set on the feature at least once #9461

Closed
sansumbrella opened this issue Mar 24, 2020 · 6 comments
Labels

Comments

@sansumbrella
Copy link
Contributor

sansumbrella commented Mar 24, 2020

@kalimar and I discovered this bug while working with a community partner adding hover-based styling of country outlines.

For that application, we call removeFeatureState in response to mouse events on the map, which occasionally caused the crash described below. This crash affects anyone using removeFeatureState, especially if they are doing so in response to user interactions where a corresponding setFeatureState may not have been processed.

mapbox-gl-js version: 1.9.0, 1.8.0, 1.8.1

browser: Chrome, Firefox

Steps to Trigger Behavior

  1. Load a map
  2. Call removeFeatureState

Note that we never called setFeatureState in the sequence above.

Link to Demonstration

https://glitch.com/edit/#!/mbx-bug-report-remove-feature-state
https://mbx-bug-report-remove-feature-state.glitch.me/

Expected Behavior

The map continues to function. Since there was no feature state to remove, it continues about its business rendering existing features.

Actual Behavior

The map crashes and ceases to be interactive.

In Chrome, this message is printed to the console on every render attempt after the removeFeatureState call:
image

Cannot convert undefined or null to object

In Firefox, a slightly more helpful error is printed to the console:
image

this.state[o][l] is undefined

Suggested fix

  1. Add a test that confirms you can call removeFeatureState without crashing if the feature state hasn't been set.

  2. Add a check that the feature being modified exists before modifying it in source_state:

if (this.state[source][feature]) {
  delete this.state[source][feature][key];
}

Workaround for users experiencing this issue

If you need to call removeFeatureState today, I recommend using a "safe" wrapper around it. For example, to remove a single feature state:

// Safe wrapper for feature state removal confirms state exists before removal
function safelyRemoveFeatureState(map, feature, state) {
    const existing = map.getFeatureState(feature);
    if (existing.hasOwnProperty(state)) {
        map.removeFeatureState(feature, state);
    }
}

// Remove the hovered state if it exists
safelyRemoveFeatureState(
    map,
    { source: "composite", sourceLayer: "admin", id: 102 },
    "hovered"
);
// Remove the glossy state if it exists
safelyRemoveFeatureState(
    map,
    { source: "composite", sourceLayer: "admin", id: 102 },
    "glossy"
);
@ryanhamley
Copy link
Contributor

Did this just start with 1.9.0? I don't see any changes to the offending file that would have caused this.

@sansumbrella
Copy link
Contributor Author

sansumbrella commented Mar 25, 2020

Nope, this has been around since at least 1.8 (and probably as long as we've had removeFeatureState). I just confirmed that it continues in 1.9.0.

In case you can't load the glitch project (I'm having trouble today), reproducing js code is in the details (GitHub doesn't like showing all the html elements as text, so I left out the accompanying html):

var map = new mapboxgl.Map({
    container: 'map', // container id
    style: 'mapbox://styles/mapbox/streets-v11',
    center: [-74.5, 40], // starting position [lng, lat]
    zoom: 9 // starting zoom
});

map.on("load", function () {
function removeSomeAbsentFeatureState() {
map.removeFeatureState({ source: "composite", sourceLayer: "admin", id: 102 }, "clicked");
}
// after some time, break the map (you can also make it happen immediately, if you like)
setTimeout(removeSomeAbsentFeatureState, 500);
});

@silberjan
Copy link

I encounter this bug in a similar context:

  1. Add a source and a layer
  2. Set feature state on any of its features
  3. Remove and re-add both (use the same ids)
  4. Try to remove the feature state

The result is the same error as as with the process of @sansumbrella

The remove and re-add is very common in our setup as we have layers that depend on a constantly changing vector tile source. As there is no way to update a vector source we always have to remove all layers, remove the source, add the source again and then add the layers again.

The error can be prevented by removing all feature state from the features of the source before removing it. I wonder if it would make sense to include this behavior in mapbox-gl (check and remove all feature state when removeSource is called)

@HansBrende
Copy link

HansBrende commented Jan 6, 2022

@ryanhamley this bug is also affecting me... The map becomes unresponsive and then I get the same stack trace as detailed above on every attempted render (every time mouse moves, scrolls, etc.), while the map is frozen. Mapbox version 2.6.1
Screen Shot 2022-01-06 at 9 51 41 AM
.

NOTE: the error does not occur for me during the removeFeatureState call (I am catching and ignoring any errors coming out of that method), but rather from inside the map's render method. (Thus causing the plethora of errors every time the mouse moves.)

@enersis-pst
Copy link
Contributor

enersis-pst commented Feb 7, 2023

same on 2.12.0
for me this happend if i remove a state which state wasnt added.

example
https://jsfiddle.net/gyqex0p5/16/

  • click somewhere
  • on drag the error will thorwn.

@stepankuzmin
Copy link
Contributor

This is fixed in #12497
It's already available in the current beta release v2.13.0-beta.1 and will be available in the upcoming final release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants