Plugin crashes when only lat or long is received

[xkilian]

2 timeseries metrics, latitude and longitude, are queried from Prometheus. Data is returned, but we currently have a quality problem with a source of data that is feeding Prometheus which causes missing values of either lat or lon. Switching the display from track map to graph made it easy to validate the quality problem. Setting manual values works. Selecting a timeframe that does not have any inconsistent data works.

The plugin should ignore values and not crash(Error message: "latlng is Null") when only lat or long is available in the time slot.

[MalteP]

The plugin also crashes in my case where lat and lon data is missing on some data points, screenshots attached. I am using InfluxDB as source.

[xkilian]

I would also suggest filtering metrics when the value is 0,0, Null or inconsistent (only lat or lon). As this will avoid having ant-paths that want to go off towrds Africa when a GPS module has no signal and returns 0,0.

[xkilian]

Is there any timeline when this issue will be looked at?

[Alkarex]

We are a bit busy at the moment, but pull request welcome!

[xkilian]

I will see if I can get someone to fix it. Will let you know.

[sebastien-coavoux]

Hi there,

I took time to take a look at it and I can share my finding. The crash occurs in leaflet lib and there is no way (as far as I can tell) that we simply try / catch this. The 3 options I can see here are :

• Make the error more user friendly, saying that one of the latitude / longitude is null (and provide the timestamp of the data ?). The crash is still here but at least it is transparent to the user

• Discard any position object if any of 2 coordinates is null. This will silently discard the data, it is not very transparent to the user but at least the rest of the data is showing. If 1 is null over several thousand, it could be acceptable.

• An mix of the 2 by adding an option into the panel to that we discard data or not. Like a box to tick in the option saying "discard position on null coordinates" . The user can choose to drop data or not, which make it more transparent overall

All of this could be done replacing the block here : https://github.com/alexandrainst/alexandra-trackmap-panel/blob/master/src/TrackMapPanel.tsx#L88

by something like :

let positions: Position[] = [];

latitudes?.forEach((latitude, i) => {
  const longitude = longitudes !== undefined ? longitudes[i] : 0;
  const popup = markerPopups !== undefined ? markerPopups[i] : `${latitude}, ${longitude}`;
  const tooltip = markerTooltips !== undefined ? markerTooltips[i] : undefined;

  // Need option 3 with if else of the 2 blocks below
  // Option 1: we throw error. Index is not very helpful
  if (typeof longitude !== 'number') {
    throw new Error(`Longitude is not a number at index ${i}`);
  }
  if (typeof latitude !== 'number') {
    throw new Error(`Latitude is not a number at index ${i}`);
  }
  positions.push({ latitude, longitude, popup, tooltip, };

  // Option 2 :  we discard
  if (typeof longitude === 'number' && typeof latitude === 'number') {
    positions.push({ latitude, longitude, popup, tooltip, };
  }
}

I would pick option 3 so that the user has the choice, but maybe I missed an obvious one.

Let me know if that looks good to you I'll be able to PR this.

[xkilian]

Here is my 2 cents:

I believe this should be option 2. The user can easily validate data using graph+data inspector if there is missing data. Option 3 is not necessary as it should be filtered by default.

The code should also drop 0,0 coordinates as these are invalid coordinates. These can also be dropped silently. I do not see how 0,0 can be valid, even for someone who lives near those coordinates as they would have more precision.

[MalteP]

I guess option 3 is the most intuitive way. If you "normalize" null values to 0, data points will at least show up somewhere without crashing Leaflet and the user can fix them. If this is expected, the user could activate the "discard position on null coordinates" option to drop invalid data.

But as a short term fix option 2 would be okay, too.

[xkilian]

Converting NULLs to Zero, seems like something that would cause more misdirection. Would informing the users fact that the plugin will drop NULL or (0,0) data in the documentation be sufficient along with an explanation on how to validate data using table or graph. Adding more options to the plugin that should be enabled by default seems not necessary.

[sebastien-coavoux]

@ZhongyuWang @Alkarex Any hot take ?

[xkilian]

Thank you Sebastien for the fix. @Alkarex let us know if this is ok or if you need any changes.

[Alkarex]

Thanks @sebastien-coavoux for the fix, and others for the sparing 👍🏻