Errors from getClusterLeaves can't be handled?

[trygveaa]

mapbox-gl-js version: 1.6.1

Question

If you call getClusterLeaves and then change the contents of the source, you will get an error saying "No cluster with the specified id.". That makes sense, however it doesn't seem to be any way to handle this error. The documentation states that the callback gets the error as the first argument, however the callback is never called, instead the error is thrown. Since this error is thrown from the worker, I can't see a way to catch it. Placing a try/catch around getClusterLeaves doesn't help (which makes sense, since it's thrown in the worker).

I see here that the error argument is hard coded to null. Shouldn't this catch the error and send it as the argument instead?

Links to related documentation

https://docs.mapbox.com/mapbox-gl-js/api/#geojsonsource#getclusterleaves

[vakila]

Hi @trygveaa thanks for using Mapbox, and for raising this question!

At first glance it looks like you're right that we're not properly passing errors through in getClusterLeaves (and other getCluser* methods) - a problem that must have been around for a couple of years, well before v1.0 let alone v1.6.1. We'll look into it.

It would be helpful if you could share a minimal example of your code that triggers this error, e.g. on a site like jsfiddle. Also, if you're willing/able to submit a PR with the fix you suggest, which sounds correct to me, we'd be more than happy to review it.

[ghost]

I'd like to append a minimal example created as a similar design to our code that was able to reproduce this issue.

https://jsfiddle.net/kef4j6x3/
(remember to update the accessToken variable with your own)

Effectively you can reproduce this trivially by calling getClusterLeaves with an invalid cluster ID which will proceed to throw an error inside the method Error: No cluster with the specified id. and never call the callback, or throw the exception to be caught by the surrounding try-catch handler.

As this method is asynchronous this is leading our team to need to add a timeout check + update to determine if the method failed.

[ryanhamley]

Submitted a fix #9251