Page MenuHomePhabricator

Cannot read property 'length' of undefined
Closed, ResolvedPublic

Description

I'm splitting this off from the parent issue.

When I go through config.yaml enabling sources and get to v3view, I hit an error

Uncommenting this source causes the error

v3view:
  public: true
  formats: [png,json,headers,svg,jpeg]
  scales: [1.3, 1.5, 2, 2.6, 3]
  static: true
  maxheight: 2048
  maxwidth: 2048
  uri: tmstyle://
  yaml:
    npm: ['@kartotherian/brighmed', "project.yml"]
  yamlSetParams:
    source: {ref: v3}

The error is

[2018-06-01T08:43:24.558Z] FATAL: tilerator/14 on maps-test2004: (levelPath=fatal/service-runner/unhandled)
    TypeError: Cannot read property 'length' of undefined
        at /srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/tilelive-tmstyle/index.js:180:56
        at Array.map (native)
        at /srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/tilelive-tmstyle/index.js:179:34
        at CassandraStore.handler.getInfo (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/@kartotherian/core/lib/sources.js:260:7)
        at /srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/tilelive-tmstyle/index.js:118:20
        at tryCatcher (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/util.js:16:23)
        at Promise.successAdapter (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/nodeify.js:23:30)
        at Promise._settlePromise (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/promise.js:566:21)
        at Promise._settlePromiseCtx (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/promise.js:606:10)
        at Async._drainQueue (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/async.js:138:12)
        at Async._drainQueues (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/async.js:143:10)
        at Immediate.Async.drainQueues (/srv/deployment/tilerator/deploy-cache/revs/9f924d6a3d8d7df882cae49a314efa13bab18186/node_modules/bluebird/js/release/async.js:17:14)
        at runCallback (timers.js:672:20)
        at tryOnImmediate (timers.js:645:5)
        at processImmediate [as _immediateCallback] (timers.js:617:5)
[2018-06-01T08:43:24.685Z]  WARN: tilerator/4 on maps-test2004: first worker died during startup, continue startup (message="first worker died during startup, continue startup", worker_pid=14, exit_code=1, startup_attempt=1, levelPath=warn/service-runner/master)

The relevant code is

//Layer ordering defined in style
opts.Layer = data.layers.map(function(layerId) {
  for(var i = 0; i < backend.data.vector_layers.length; i++) {
	var layer = backend.data.vector_layers[i];
	if(layer.id == layerId) {
	  return layerToDef(layer);
	}
  }
});

Although tilelive-tmstyle needs better error reporting, the fix isn't just changing something in its code, since it's supposed to be doing something. This section of code is *supposed* to go through the layers the tmstyle calls for and match them up against source layers.

I have some thoughts on approaches for possible fixes and debugging, but I'll post that in a comment to keep it separate.

There are a few monkey patches in place here to get some other things working, which are

To work around T196124: Logged errors should provide a full stack trace and get a stack trace

--- a/node_modules/service-runner/lib/logger.js
+++ b/node_modules/service-runner/lib/logger.js
@@ -202,7 +202,7 @@ class Logger {
         const logUnhandledException = (err) => {
             if (!inLogger) {
                 inLogger = true;
-                this.log('fatal/service-runner/unhandled', err);
+                this.log('fatal/service-runner/unhandled', err.stack);
                 inLogger = false;
             }
         };

To get a tilejson that the .tm2 yaml could point at, which we believed was important

--- a/node_modules/@kartotherian/brighmed/project.yml
+++ b/node_modules/@kartotherian/brighmed/project.yml
@@ -22,7 +22,7 @@ format: "png"
 minzoom: 0
 maxzoom: 19
 srs: "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over"
-source: "tmsource:///home/user/path/to/meddo"
+source: "tilejson+file:///srv/deployment/tilerator/deploy/node_modules/@kartotherian/brighmed/tile.json"
 styles:
   - style.mss
   - water.mss
--- /dev/null
+++ b/node_modules/@kartotherian/brighmed/tile.json
@@ -0,0 +1 @@
+{"tilejson":"2.1.0","center":[-122.4144,37.7907,14],"description":"Data source for Wikimedia's fork of osm-bright2","format":"pbf","vector_layers":[{"id":"
landcover"},{"id":"landuse"},{"id":"waterway"},{"id":"water"},{"id":"pier"},{"id":"building"},{"id":"transport"},{"id":"aeroway"},{"id":"park"},{"id":"bound
ary"},{"id":"country_name"},{"id":"place"},{"id":"state_name"},{"id":"transport_name"},{"id":"landcover_name"},{"id":"transit_stop"}],"maxzoom":14,"minzoom"
:0,"name":"osm-pbf","attribution":"<a href=\"https://wikimediafoundation.org/wiki/Maps_Terms_of_Use\">Wikimedia maps</a> | Map data &copy; <a href=\"http://
openstreetmap.org/copyright\">OpenStreetMap contributors</a>","tiles":["https://maps.wikimedia.org/osm-pbf/{z}/{x}/{y}.pbf"]}

To disable sources to identify the problem, all sources after v3view were commented out

Event Timeline

Pnorman triaged this task as Normal priority.Jun 1 2018, 9:09 AM
Pnorman created this task.
Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptJun 1 2018, 9:09 AM

@mojodna, @Mholloway, and myself were looking at fixing this by adding a tilejson, but I'm not sure that's the right approach.

We looked carefully at the tm2(style) differences between brighmed and osm-bright.tm2, but we didn't give the same scrutiny to meddo and osm-bright.tm2source, and I wonder if the issue could lie there. If there is an important difference, it should be in a top-level property, or in each layer, but it shouldn't be in the Datasource portion of the layer.

A bit of python gets the top level keys

python -c 'from yaml import safe_load; print(safe_load(file("data.yml")).keys())'

Meddo

['Layer', 'attribution', 'description', '_parts', 'center', '_prefs', 'minzoom', 'maxzoom', 'name']

osm-bright.tm2source

['Layer', 'attribution', 'description', 'center', '_prefs', 'minzoom', 'maxzoom', 'name']

_parts is internal, used for simplifying YAML, and shouldn't matter because it's *additional*.

Similar python gets the keys for the first layer

python -c 'from yaml import safe_load; print(safe_load(file("data.yml"))["Layer"][0].keys())'

meddo

['geometry', 'properties', 'id', 'Datasource']

osm-bright.tm2source

['srs', 'properties', 'description', 'fields', 'id', 'Datasource']

srs, description, fields are believed to be optional, but is this true? For the first osm-bright.tm2source layer they're

description: ''
fields: 
  class: String
  osm_id: Number
  way_area: Number
  z_order: Number
srs: +proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over

I wonder if the lack of a description and field list is causing it to not load properly and match up?

Mholloway added a comment.EditedJun 1 2018, 5:41 PM

After spending the morning trying to wrap my non-maps-expert head around this: it looks like vector_layers is considered a required field in this context (to be formalized), the vector_layers field is generated by the source's consumer (T160025), and resolving T160025 might take care of this (as suggested at the conclusion of Paul's comment above)?

(And yeah, if vector_layers is indeed required, as seems to be the assumption upstream, would be nice to have a check with meaningful error output.)

After spending the morning trying to wrap my non-maps-expert head around this: it looks like vector_layers is considered a required field in this context (to be formalized), the vector_layers field is generated by the source's consumer (T160025), and resolving T160025 might take care of this (as suggested at the conclusion of Paul's comment above)?

I don't think tilejson is related to this, because the style loads correctly above (genview), the style has a tilejson, and the tilejson has a vector_layers field. I think this is all about the tm2source and tm2 formats.

Adding descriptions and fields in https://github.com/kartotherian/meddo/pull/16, and I'll rebuild and test with it

So, I had this working earlier tonight. I came back after dinner and worked on other parts of the instructions, and now it's now not working. I checked that I'm not crazy, and https://phabricator.wikimedia.org/T194106#4255608 was done from maps-test2004, and my console log shows I tested tilerator too.

I checked what I was deploying, and the deploy repo was 074d01a, and the scap log shows it deployed without error. I checked the exact same commit out, and now it's failing.

That I might just be very confused always goes without saying. ;) Bummer this went away and then returned. I'll clear some more other stuff off my plate and then look at it again, systematically and starting from the beginning.

This is now believed to be fixed.

The cassandra store saves the metadata of its generating layer at keyspace creation time. This is poorly documented, with the instructions on the page not working, nor making clear the critical importance of proper keyspace setup in order to even *run* the stack.

Pnorman moved this task from Backlog to In progress on the Maps-Sprint board.Jun 12 2018, 9:14 PM
Pnorman closed this task as Resolved.Jun 13 2018, 4:48 AM
Pnorman claimed this task.
Vvjjkkii renamed this task from Cannot read property 'length' of undefined to yubaaaaaaa.Jul 1 2018, 1:06 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed Pnorman as the assignee of this task.
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: Aklapper, gerritbot.
WhitePhosphorus renamed this task from yubaaaaaaa to Cannot read property 'length' of undefined.Jul 1 2018, 2:48 AM
WhitePhosphorus closed this task as Resolved.
WhitePhosphorus assigned this task to Pnorman.
WhitePhosphorus lowered the priority of this task from High to Normal.
WhitePhosphorus updated the task description. (Show Details)
WhitePhosphorus added subscribers: Aklapper, gerritbot.