mapbox / mapnik-omnivore

Node module that returns metadata about spatial files.
44 stars 19 forks source link

Don't swallow error messages #125

Open wilhelmberg opened 9 years ago

wilhelmberg commented 9 years ago

Debugging a Mapbox Studio Classic and a KML problem, I came across this error message

image

which led me to believe the data was faulty and investigate in that direction, when in reality it seems that in the context of Mapbox Studio Classic and KMLs mapnik-omnivore/node-srs/node-gdal are not able to find the gdal support files:

image

May I suggest to augment the custom error message with the original error message? e.g. change

return invalid('OGR source missing extent');

to

return invalid('OGR source missing extent: ' + err);

https://github.com/mapbox/mapnik-omnivore/blob/ce7f47af9135acdc144991d5ba27469d76e5649f/lib/ogr.js#L55

/cc @GretaCB

wilhelmberg commented 9 years ago

I haven't looked through the whole code, but it would be really great, if all similar error handling routines could be changed (if there are any others :smirk:)

springmeyer commented 9 years ago

The error message suppression is intentional. My understanding is that @rclark and @GretaCB decided to do this in order to sanitize/collapse all Mapnik and GDAL errors down to very simple messages that are also "Safe" and do not expose user info. But I definitely agree with you @BergWerkGIS that this adds an annoying friction and slowness to debugging for us - the developers trying to figure out where the generic error message is coming from. I think we should revisit this overall. One idea would be to add a verbose mode to mapnik-om such that all the raw Mapnik and GDAL errors that are suppressed are printed to stderr along with the err.stack.

mannylopez commented 8 years ago

I don't know if this is related but when trying to upload run.gpx I also get Error: OGR source missing extent.

I was able to upload the same file via the uploads page so the data is good.

mannylopez commented 8 years ago

Digging a bit deeper it looks like https://github.com/mapbox/mapbox-studio-classic/issues/1473 is a better place to post this. Sorry for the noise!

GretaCB commented 8 years ago

Thanks @mannylopez , I can use run.gpx as a test-case to clean this up a bit. Happen to have any other files you know that run into this error?

hey @BergWerkGIS , what file were you using to run into this error? I can take a look at some test-cases and roll something new out with the original error (if it doesn't reveal sensitive server info).

GretaCB commented 8 years ago

@springmeyer This could be great:

I think we should revisit this overall. One idea would be to add a verbose mode to mapnik-om such that all the raw Mapnik and GDAL errors that are suppressed are printed to stderr along with the err.stack.

I can play around with this.