Esri/esri-leaflet

Feature layer _activeRequests count is wrong when setWhere is called with pending requests

Voileexperiments opened this issue · 7 comments

Describe the bug

I was debugging a issue where the load event of a feature layer is never called after setWhere is called frequently in succession.

The cause is here:

if (this.options.where !== originalWhere) {
return;
}

Every request increments _activeRequests, but if where has changed before the response comes back then the the request will not decrement _activeRequests (in _postProcessFeatures). load event only fires if _activeRequests is non-positive, so it will never fire again after this happens.

It seems to me there are two possible fixes:

  1. Set _activeRequests = 0 in setWhere (buggy, what if setWhere is called many times before moving back to original value?)
  2. Change _activeRequests to store request objects instead (Set or Map) with a proper key, then count its size. Then requests are properly invalidated when setWhere happens

In my use case, where will be set frequently (more frequent than before requests are guaranteed to come back) and so throttling the use of setWhere isn't an option.

Reproduction

From the standard jsbin, run this:

parks.on('load', function(ev) { console.log('Called at ' + Date.now()); })

After some zoom/pan operations and waiting for requests to complete, parks._activeRequests should be 0 and there should be a log entry.

Then run this:

parks.setWhere('1=0');
parks.setWhere('1=1');

parks._activeRequests will stay at a positive number like 5 indefinitely, and load event will never be called after zooming or panning.

Logs

No response

System Info

Latest leaflet version

Additional Information

No response

Thank you for the report. For your replication case can you please post a link to the jsbin example and also the "expected: ____" and "actual: ____" behaviors to help us understand the specific issue you are reporting. Thanks!

can you please post a link to the jsbin example and also the "expected: ____" and "actual: ____" behaviors

I already provided the details of the issue, reproduction steps and and some analysis in the original post. jsbin is not provided because the reproduction is not that simple. I'm maintaining an application using this library that performs various complex operations when user interacts with it, which means most of the issues I encounter require user interaction and is not automate-able. (It also requires me to dig in the library to figure out the root cause, but that's another matter.)

We appreciate the debugging you have done and some potential solutions you have proposed. But, going back to the original problem statement

I was debugging a issue where the load event of a feature layer is never called after setWhere is called frequently in succession.

we want to understand the context of what is the expected behavior when listening the load event on the layer, compared to the actual behavior of the load event in your usage and thus how it impacts your application.

we want to understand the context of what is the expected behavior when listening the load event on the layer, compared to the actual behavior of the load event in your usage and thus how it impacts your application.

This is irrelevant; the issue here is that this breaks load event in an unrecoverable way because load event assumes that _activeRequests should behave properly. As long as someone registers any handler to load event relying on the event actually being fired in expected manner, they will be hit by this issue. (I haven't checked loading event, but I believe it suffers from the same issue.)

Anyways, the detail of what I originally was doing isn't really important; it was only mentioned to demonstrate that I am currently encounter actual issues in my application as a result. The reproduction steps are there to show a minimal example in which this will cause unexpected behaviour.

stale commented

This issue has been automatically marked as stale because we're waiting on more information or details, but have not received any response. It will be closed if no further activity occurs. Thank you!

stale commented

This issue has been automatically marked as stale because we're waiting on more information or details, but have not received any response. It will be closed if no further activity occurs. Thank you!