maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
6.3k stars 686 forks source link

Geolocate Icontrol continuously throws “Unexpected watchState BACKGROUND_ERROR” #2294

Open DevelWolf opened 1 year ago

DevelWolf commented 1 year ago

maplibre-gl-js version: 2.4.0 browser: n/a sourcefile ./src/ui/control/geolocate_control.ts ~ line 195

Steps to Trigger Behavior

Cannot provide a demonstration because error depends on browser's geolocation source.

Actual Behavior

In method _setErrorState(): First geolocation error (i.e. timeout) shifts _watchState from 'BACKGROUND' to 'BACKGROUND_ERROR'. Following geolocation errors run into default, because there is no case 'BACKGROUND_ERROR', throwing an exception.

Fix

Add a case 'BACKGROUND_ERROR' to the switch, at BACKGROUND or at ACTIVE_ERROR

HarelM commented 1 year ago

It's hard to understand without being able to reproduce and see the issue. But if you can reproduce it easily and send a PR that fixes this, I'll be happy to review it.

DevelWolf commented 1 year ago

If while in BACKGROUND mode the geolocation continuously runs into timeouts, which I don't know how to force, the _setErrorState() method continuously throws exceptions:

img1 counting ... img2

This is caused by the ommision of a case "BACKGROUND_ERROR" in src/ui/control/geolocate_control.ts:

    _setErrorState() {
        switch (this._watchState) {
            case 'WAITING_ACTIVE':
                this._watchState = 'ACTIVE_ERROR';
                this._geolocateButton.classList.remove('maplibregl-ctrl-geolocate-active');
                this._geolocateButton.classList.add('maplibregl-ctrl-geolocate-active-error');
                break;
            case 'ACTIVE_LOCK':
                this._watchState = 'ACTIVE_ERROR';
                this._geolocateButton.classList.remove('maplibregl-ctrl-geolocate-active');
                this._geolocateButton.classList.add('maplibregl-ctrl-geolocate-active-error');
                this._geolocateButton.classList.add('maplibregl-ctrl-geolocate-waiting');
                // turn marker grey
                break;
            case 'BACKGROUND':
                this._watchState = 'BACKGROUND_ERROR';
                this._geolocateButton.classList.remove('maplibregl-ctrl-geolocate-background');
                this._geolocateButton.classList.add('maplibregl-ctrl-geolocate-background-error');
                this._geolocateButton.classList.add('maplibregl-ctrl-geolocate-waiting');
                // turn marker grey
                break;
            case 'ACTIVE_ERROR':
                break;
            default:
                throw new Error(`Unexpected watchState ${this._watchState}`);
        }

Obvious fix is to add the missing case:

case 'ACTIVE_ERROR':
case 'BACKGROUND_ERROR':

I've patched this in my copy of maplibre-gl.js, making the annoying browser log entries disappear.

HarelM commented 1 year ago

I personally implemented something similar which works a lot better using angular, having said that, from what i can read in the code the following can cause this issue, I think:

  1. Start geolocation tracking
  2. Receive a position outside the map
  3. Receive another position. I've seen the out of map bounds check in the onSuccess callback. If this is what's causing the problem then your fix might do the trick, but if you can't narrow down the issue adding this line of code is not the right solution as we don't know what we are fixing. My suggestion for you is to try and better understand what causes this issue, then write a small test to reproduce it and then fix the code in the right place. Once you can do that open a PR and I'll be happy to merge it 😀
DevelWolf commented 1 year ago

Sorry, but I don't know a way to programmatically force Geolocation failures, especially timeouts. I have provided everything I know and think; hopefully another one can take care of this bug.

HarelM commented 1 year ago

What framework are you using to add the control? It seems that you are adding it again and again and then calling trigger then receiving and error from the geolocation. I'm not sure this is a problem with this library, but rather the library that wraps it. I tried writing a test to simulate getting to this invalid state and couldn't...

HarelM commented 9 months ago

I would recommend creating a version with more console log in order to better understand the root cause of this issue. If this is not possible, then there's no good way for us to help solve the issue, the code there is pretty straight forward although complicated due to all the possible states...