CartoCSS errors when instantiating named maps

[mojodna]

The following named map configuration (which should work against any account since it doesn't reference any tables) triggers the below error, which a) requires excessive parsing and b) includes error messages referencing line numbers that don't exist.

{
  "name": "CartoDB_Basemaps",
  "version": "0.0.1",
  "layergroup": {
    "version": "1.0.1",
    "minzoom": 0,
    "maxzoom": 20,
    "layers": [
      {
        "type": "mapnik",
        "name": "global_variables",
        "options": {
          "cartocss_version": "2.1.1",
          "sql": "SELECT '', null::geometry AS the_geom_webmercator LIMIT 0\n",
          "cartocss": "@polygoncolor: #ffd479;\n@cachebuster: #0000c7;\n@water: #cdd2d4;\n\n\nMap {\n\tbackground-color: @water;\n  bufferz-size: 256;\n}\n\n@landmass_fill: lighten(#e3e3dc, 8%);\n@landmass_line: darken(#bfc7c8, 10%);\n\n@greenareas_fill_low: lighten(#d4dad6,8%);\n@greenareas_fill_medium: lighten(#d4dad6,5%);\n@greenareas_fill_high: lighten(#d4dad6,6%);\n\n@buildings: lighten(#e3e3dc, 5%);\n@buildings_outline_16: #ddd;\n@buildings_outline: #ddd;\n\n@aeroways: #e8e8e8;\n\n@ne10roads_line_color: white;\n@ne10roads_line_outline: darken(#b4b2a3, 2%);\n@ne10roads_7_minor_color: darken(@ne10roads_line_color,12%);\n\n@ne_rivers_stroke: lighten(#346fa1,30%);\n@ne_rivers_casing: darken(#f5f5f3,1%);\n\n@urbanareas: darken(#f5f5f3, 4%);\n@urbanareas_highzoom: darken(#f5f5f3, 3%);\n\n\n@admin0_4: lighten(#c79297, 20%);\n@admin0_5: lighten(#c99297,10%);\n@admin0_6: mix(lighten(#c99297, 20%), lighten(#e3e3dc, 8%), 20);\n@admin0_7: lighten(#c99297,20%);\n\n@admin1_lowzoom: lighten(#6d6e71, 40%);\n@admin1_highzoom: lighten(#c79297, 15%);\n\n@admin1_labels: #ccc;\n@admin1_labels_halo: white;\n\n//osm roads\n@rail_line: #dddddd;\n@rail_dashline: #fafafa;\n\n@osm_roads_z9_highway_casing: #ccc;\n@osm_roads_z9_highway_stroke: white;\n@osm_roads_z9_major_stroke: #d3d3d3;\n\n@osm_roads_z10_highway_casing: #ccc;\n@osm_roads_z10_highway_stroke: white;\n@osm_roads_z10_major_stroke: #ccc;\n@osm_roads_z10_minor_stroke: #ddd;\n\n@osm_roads_z11_highway_casing: #ccc;\n@osm_roads_z11_highway_stroke: white;\n@osm_roads_z11_major_stroke: #d4d4d4;\n@osm_roads_z11_minor_stroke: #ddd;\n\n@osm_roads_z12_highway_casing: #c4c4c4;\n@osm_roads_z12_highway_stroke: white;\n@osm_roads_z12_major_casing: #d9d9d9;\n@osm_roads_z12_major_stroke: #fefefe;\n@osm_roads_z12_minor_stroke: #ddd;\n\n@osm_roads_z13_highway_casing: #c0c0c0;\n@osm_roads_z13_highway_stroke: white;\n@osm_roads_z13_major_casing: #ccc;\n@osm_roads_z13_major_stroke: #fcfcfc;\n@osm_roads_z13_minor_stroke: #ddd;\n\n@osm_roads_z14plus_highway_casing: #bbb;\n@osm_roads_z14plus_highway_stroke: white;\n@osm_roads_z14plus_major_casing: #c4c4c3;\n@osm_roads_z14plus_major_stroke: white;\n@osm_roads_z14plus_minor_casing: #ddd;\n@osm_roads_z14plus_minor_stroke: #f9f9f9;\n\n@osm_roads_path_stroke: #eee;\n@osm_tunnel_stroke: #eee;\n\n// labels\n@label_foreground_fill: #8494a1;\n@label_foreground_halo_fill: rgba(236,236,234,0.7);\n@label_background_fill: #b6b6b6;\n@label_background_halo_fill: rgba(255,255,255,0.7);\n@labels_lowzoom_shield_fill: lighten(@label_foreground_fill, 7%);\n@labels_lowzoom_shield_halo_fill: lighten(@label_foreground_halo_fill,10%);\n@labels_highzoom_text_fill: lighten(@label_foreground_fill,15%);\n@labels_highzoom_halo_fill: lighten(@label_foreground_halo_fill,10%);\n\n@labels_highzoom_class1_text_fill: lighten(@label_foreground_fill,5%);\n@labels_highzoom_class2_text_fill: darken(@label_foreground_fill,5%);\n\n@labels_marine_fill: white;\n@labels_marine_halo_fill: lighten(@label_foreground_fill,10%);\n\n@osm_roads_labels_fill: #bbb;\n@osm_roads_labels_halo: white;\n\n// assets\n@city_shield_file: url(\"http://s3.amazonaws.com/libs.cartocdn.com/stamen-base/city_shield_light.svg\");\n@city_shield_file_lowzoom: url(\"http://s3.amazonaws.com/libs.cartocdn.com/stamen-base/city_shield_light.svg\");\n@capital_shield_file: url(\"http://s3.amazonaws.com/libs.cartocdn.com/stamen-base/capital_shield_light.png\");\n@capital_shield_file_lowzoom: url(\"http://s3.amazonaws.com/libs.cartocdn.com/stamen-base/capital_shield_light.png\");\n\n@label_park_halo_fill: lighten(#e3e3dc, 8%);\n@label_park_fill: darken(#d4ded6, 30%);\n\n@label_water_halo_fill: lighten(#e3e3dc, 8%);\n@label_water_fill: lighten(#6b8a95, 5%);\n\n@park_texture_opacity: 0.0;\n"
        }
      },
      {
        "type": "mapnik",
        "name": "false_background",
        "options": {
          "cartocss_version": "2.1.1",
          "sql": "SELECT the_geom_webmercator\nFROM false_background_zoomed('!scale_denominator!', !bbox!) AS _\n",
          "cartocss": "#false_background {\n  polygon-fill: @landmass_fill; \n}\n"
        }
      }
    ]
  }
}

{
  "errors": [
    "Error: style1:4:56 Rule bufferz-size not allowed for Map.\nstyle0:8:2 Unrecognized rule: bufferz-size. Did you mean buffer-size?"
  ]
}

(Yes, bufferz-size is an intentional typo.)

Excessive parsing: splitting the newlines for each item in errors, stripping the Error: prefix, determining which layer corresponds to styleN (it's not as simple as treating N as the layer index, as layers without CartoCSS don't increase the style count).

style0:8:2 Unrecognized rule: bufferz-size. Did you mean buffer-size? is correct.

I'm not sure what's triggering style1:4:56 Rule bufferz-size not allowed for Map; it doesn't correspond to anything in the map config. Given the column index (56), I suspect that a flattened (newlines stripped) version of the first stylesheet is being appended to the second (but why is the error message different?). (This is further supported by seeing repeated errors from maps with many more layers; to parse them, I took each line from each error item and pulled the unique values out.)

[rochoa]

The problem is how carto build the error messages: https://github.com/CartoDB/carto/blob/master/lib/carto/parser.js#L243-L250

What would work better for you? Having an array of error messages? Having an per layer array with errors per layer?

About the Map in style1 (layer 1) that's because we apply Map rules for all layers instead of per layer :disappointed:.

I guess you are getting those errors at named map instantiation, right? Named map template should be validated on creation/update time so we could avoid a lot of errors in run time: https://github.com/CartoDB/Windshaft-cartodb/issues/134

This is kind of similar to https://github.com/CartoDB/Windshaft-cartodb/issues/84

[rochoa]

I added a test with a simplified version of the map configuration: https://github.com/CartoDB/Windshaft/commit/f789e642f89e0ef3c372a53536cce140cc64d396

[mojodna]

I've seen boilerplate like this, originally from https://github.com/mapbox/carto#from-code. Not sure how that plays into things.

if (Array.isArray(err)) {
  err.forEach(function(e) {
    carto.writeError(e, options);
});

What would work better for you? Having an array of error messages? Having an per layer array with errors per layer?

Probably array of messages (parsed, with the location separate from the message) per layer, keyed by layer name.

I guess you are getting those errors at named map instantiation, right? Named map template should be validated on creation/update time so we could avoid a lot of errors in run time: #134

Yup, but I'm working around it by immediately instantiated a map. It'd be nice to eliminate that. It'd also be nice to be able to not have to check for the existence of a named map before deciding whether to POST or PUT (with the name, which is also in the payload). Not sure what makes the most sense there though...