Fixing matrix errors

[hyperknot]

Failed to invert matrix errors are quite bad, as they get the UI in a "stuck" state, where no interaction have any effect. Effectively the map part of the application is frozen.

I've been debugging how do they happen using Sentry in a live production site, as otherwise I couldn't reproduce it on my machine, but they happen very regularly to users (across all kind of browsers).

There are multiple ways a map can get into this illegal state, here is the most common one:

1. scroll_zoom / _onScrollFrame / delta !== 0 is not run first. This means that this._targetZoom is not initialized.
   

   mapbox-gl-js/src/ui/handler/scroll_zoom.js

   Line 207 in b9e32bf

   if (this._delta !== 0) {

2. interpolate receives a value with undefined as the second parameter. The way interpolate is written, it'll turn this into NaN, even if k is 0.
   

   mapbox-gl-js/src/ui/handler/scroll_zoom.js

   Line 235 in b9e32bf

   tr.zoom = interpolate(this._startZoom, this._targetZoom, k);

3. geo / transform / set zoom has a guard for making sure a value is between min and max zoom, but this guard doesn't protect against NaN.
   

   mapbox-gl-js/src/geo/transform.js

   Line 161 in b9e32bf

   const z = Math.min(Math.max(zoom, this.minZoom), this.maxZoom);

4. NaN goes through setZoom and just kills the matrix calculations.

Recommendations:

1. By far the safest and simplest option is to guard set zoom. This'll protect against all kind of errors, not just this. I recommend adding an

if (typeof zoom !== 'number' || !isFinite(zoom)) return

guard at the first line.

2. Interpolate could return a value without calculation if t is 0 or 1, both as an optimization as well as making it more reliable.

3. scroll_zoom should be fixed so that _targetZoom and _easing is always initialised.

@anandthakker @mourner @jfirebaugh

[korywka]

We are getting same errors after upgrade to v45:

Downgrading to 44 😞

[andrewharvey]

see also #6486

[hyperknot]

@Bravecow I think this was present in 0.44 as well, look at #6486.
#6029 appeared in 0.44.0 I believe.

[hyperknot]

Two more paths for an illegal matrix error.

1. This line:
   

   mapbox-gl-js/src/ui/handler/scroll_zoom.js

   Line 244 in b9e32bf

   tr.zoom = this._targetZoom;

   
   can set zoom to undefined when _targetZoom is not yet set. undefined gets turned into NaN after the min-max guard, which again sets zoom to NaN.

2. _easing() is not present at this point:
   

   mapbox-gl-js/src/ui/handler/scroll_zoom.js

   Line 234 in b9e32bf

   const k = this._easing(t);

Causes an exception and later an assertion error, but I believe this doesn't propagate to illegal matrix error.

[hyperknot]

What would you like to be the solution here? I can make a PR for adding

if (typeof zoom !== 'number' || !isFinite(zoom)) return

to set zoom. This would solve all the problems in practice, as a quick term solution.

A better solution of revised scroll logic should still be considered in the future though.