mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.22k stars 2.23k forks source link

Uncaught TypeError: Cannot read property 'length' of null (CollisionTile in Tile.reloadSymbolData) #3700

Closed sguignot closed 7 years ago

sguignot commented 7 years ago

mapbox-gl-js version: 0.28 Not reproduced with 0.26 ; Maybe present in 0.27 too.

Steps to Trigger The Error

  1. Setup a geojson layer with a lot of features: for example, every building polygons from OSM in a city like Paris, France.
  2. Zoom in and out randomly inside the city
  3. Sometimes the following error occurs (details and stack below): Uncaught TypeError: Cannot read property 'length' of null

Error

Uncaught TypeError: Cannot read property 'length' of null
    at new CollisionTile
    at Tile.reloadSymbolData
    at Tile.done
    at Actor.receive
var CollisionTile = function CollisionTile(angle, pitch, collisionBoxArray) {
    if (typeof angle === 'object') {
        var serialized = angle;
        collisionBoxArray = pitch;
        angle = serialized.angle;
        pitch = serialized.pitch;
        this.grid = new Grid(serialized.grid);
        this.ignoredGrid = new Grid(serialized.ignoredGrid);
    } else {
        this.grid = new Grid(EXTENT, 12, 6);
        this.ignoredGrid = new Grid(EXTENT, 12, 0);
    }

    this.minScale = 0.5;
    this.maxScale = 2;

    this.angle = angle;
    this.pitch = pitch;

    var sin = Math.sin(angle),
        cos = Math.cos(angle);
    this.rotationMatrix = [cos, -sin, sin, cos];
    this.reverseRotationMatrix = [cos, sin, -sin, cos];

    // Stretch boxes in y direction to account for the map tilt.
    this.yStretch = 1 / Math.cos(pitch / 180 * Math.PI);

    // The amount the map is squished depends on the y position.
    // Sort of account for this by making all boxes a bit bigger.
    this.yStretch = Math.pow(this.yStretch, 1.3);

    this.collisionBoxArray = collisionBoxArray;
    if (collisionBoxArray.length === 0) { // <===== !!!! CRASH HERE !!!!
        // the first collisionBoxArray is passed to a CollisionTile
stack_0 stack_1 stack_2
mollymerp commented 7 years ago

@sguignot thanks for reporting this issue! could you please provide a jsfiddle / jsbin that reproduces this error?

it is possible that https://github.com/mapbox/mapbox-gl-js/pull/3681 fixes this exception, so I'd like to verify before investigating further.

thanks!

lbud commented 7 years ago

👍 after some investigation, I suspect this is a different issue from #3681, but we haven't been able to reproduce it so far 😬 a jsfiddle doing so would be very helpful!

lucaswoj commented 7 years ago

Closing pending reproduction steps

sguignot commented 7 years ago

mapbox-gl-js versions: 0.27, 0.28, 0.29, 0.30 Not reproduced with 0.26, this bug is present in all versions since 0.27.

Steps to Trigger The Error

  1. Get a mouse device that supports scrolling with high speed / velocity / inertia (I am using a Magic mouse 2 from Apple: http://www.apple.com/shop/product/MLA02LL/A/magic-mouse-2).
  2. Go to the following blog demo: https://www.mapbox.com/bites/00304/
  3. Open the browser console and add "length" as error filter to avoid the flood of 404 errors.
  4. Quickly move the camera where features are located: zoom in and out quickly, pan and change the camera pitch. In less than a minute of zooming and moving the camera I reproduce the following error in the console: Uncaught TypeError: Cannot read property 'length' of null (browsers on which I reproduce this error: Google Chrome 55 and Safari 10).

Error

Uncaught TypeError: Cannot read property 'length' of null
    at new CollisionTile
    at Tile.reloadSymbolData
    at Tile.done
    at Actor.receive
screen shot 2017-01-10 at 15 04 40

Please re-open, this issue is blocker for us (it occurs more often than in your demo).

musicformellons commented 7 years ago

I am getting these as well (version 0.30.0):

Uncaught TypeError: Cannot read property 'length' of null
    at new CollisionTile (eval at <anonymous> (0.50f3799….js:263), <anonymous>:157:670)
    at Tile.reloadSymbolData (eval at <anonymous> (0.50f3799….js:263), <anonymous>:101:1495)
    at Tile.t (eval at <anonymous> (0.50f3799….js:263), <anonymous>:101:2104)
    at Actor.receive (eval at <anonymous> (0.50f3799….js:263), <anonymous>:213:738)
CollisionTile @ mapbox-gl.js?31ef:157
Tile.reloadSymbolData @ mapbox-gl.js?31ef:101
t @ mapbox-gl.js?31ef:101
Actor.receive @ mapbox-gl.js?31ef:213

And I am indeed using a geojson layer.

mourner commented 7 years ago

@sguignot can you please try reproducing this again and posting the properties of this (Tile instance) on the breakpoint where it happens (in either done or reloadSymbolData)? I'm specifically interested in the values of this.state and this.collisionBoxArray.

sguignot commented 7 years ago

@mourner sorry for the delay. I ran the demo "Restaurants Complaints NYC" with https://api.tiles.mapbox.com/mapbox-gl-js/v0.30.0/mapbox-gl-dev.js I put conditional breakpoint on line 49 of file collision_tile.js (condition is !collisionBoxArray) line 49: if (collisionBoxArray.length === 0) {

When the error occurs (ie !collisionBoxArray), I have this.state : "reloading" and this. collisionBoxArray : null. You will find the detailed context for each frame of the call stack below:

mapbox_stack0_collitiontile_ctor mapbox_stack1_reloadsymboldata mapbox_stack2_done
sguignot commented 7 years ago

@mourner it seems a lot easier to reproduce this error when spinning the map using the right click, check out the following screencast: mapbox-gl-js_issue_3700_howto_reproduce_short

mourner commented 7 years ago

@sguignot finally managed to reproduce, thanks a lot for the help! On it now.

GuillaumeTS commented 7 years ago

Reproduced with the same steps with mapbox 0.37 and 0.38.

lbud commented 7 years ago

I can also reproduce on master — will produce a minimal example.

lbud commented 7 years ago

https://jsbin.com/sujonoyupa/edit?html,output — error appears after clicking the button a few times.

sguignot commented 7 years ago

@lbud Thanks for the jsbin, do you have any news regarding the bugfix?

We have a lot of logs/alerts in production because of this regression, it would be great to have a fix in the next version (v0.39).

edap commented 7 years ago

I'm having the same problem in production

mollymerp commented 7 years ago

@sguignot @edap would you be willing to see if the error is still happening on master? I think it was fixed in https://github.com/mapbox/mapbox-gl-js/commit/caf4cfb058b5060dffee5ccb7795a9b5add7d20a as I can no longer reproduce on master.

mollymerp commented 7 years ago

Closing for now – please reopen if you continue to see this issue after the next release (this afternoon)

brncsk commented 7 years ago

This is still happening for me on 0.39.1.

sguignot commented 7 years ago

Thanks @mollymerp it's fixed on my side, I just tested with 0.39.1 and errors are gone.

For quick testing, you can reproduce the error in 0.38 here after a few clicks: https://jsbin.com/sonuvoyofe/1/edit?html,output

Here in 0.39.1 you can check error is gone: https://jsbin.com/hoyaputude/1/edit?html,output

I'm going to deploy 0.39.1 in production soon, then after a few days I will confirm if the error is completely gone or if it persists (there may be several root causes after all).

EbilPanda commented 7 years ago

yeah also happening to me occasional when zooming or using the fly-method... I'm using custom tileset and style.

Error:

ERROR TypeError: Cannot read property 'length' of null at new CollisionTile (mapbox-gl.js:337) at Function.CollisionTile.deserialize (mapbox-gl.js:337) at Tile.reloadSymbolData (mapbox-gl.js:215) at mapbox-gl.js:215 at Actor.receive (mapbox-gl.js:397) at t.invokeTask (polyfills.js:3) at Object.onInvokeTask (core.es5.js:4140) at t.invokeTask (polyfills.js:3) at r.runTask (polyfills.js:3) at Worker.invoke (polyfills.js:3)

using 0.39.1 via npm, with webpack 3.4.1 and excluded mapbox from uglifier (exports.module.noParse = /(mapbox-gl)\.js$/;) in an ionic2-app (angular4)

mollymerp commented 7 years ago

@EbilPanda @brncsk sorry you're still running into this – can you cut a new issue with a jsfiddle/jsbin that can reproduce the issue?

EbilPanda commented 7 years ago

i'm sorry I'm currently not able to reproduce the error on fiddl/bin :( I think the problem is with webpack/uglify that stills running over the already uglified code and then occures error, I think I will go with the cdn to test if it happends there too

EbilPanda commented 7 years ago

But i found the error line in the mapbox-gl-devjs: Line 23052 if (collisionBoxArray.length === 0) I've attached to scope-variables directly before the error occured (Line 23051). mapbox.scope.json.txt

shf123 commented 7 years ago

It also still happens for me in version 0.39.1.

I made an example in jsfiddle: https://jsfiddle.net/stighef/y5z4cwLt/

A FeatureCollection with 10 000 rectangles are toggled on and off when the zoomend-event is triggered.

After zooming out and in, I get lots of these:

image

sguignot commented 7 years ago

@mollymerp I reopen the issue because the jsfiddle from @shf123 triggers the error with version 0.39.1

snkashis commented 7 years ago

Yeah, still an issue for me in 0.39.1 as well.

apolishch commented 7 years ago

Yeah happening for me on 0.39.1 as well

mourner commented 7 years ago

Thanks a lot for the new test case — got it to immediately reproduce! Will make fixing much easier.

mourner commented 7 years ago

Hey everyone, we have a fix in #5185 — until we ship this in a release, feel free to use this patch for a custom build of GL JS if you experience this problem.