From 30272924a907b4d557e45932bf3cc3d392d5e85b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 16 Jul 2018 14:20:09 +0200 Subject: [node] capture invalid input data as `ParseError` objects This allows us to distinguish them from other types of errors --- platform/node/src/node_map.cpp | 37 +++++++++++++++++++++++++++----- platform/node/src/node_map.hpp | 3 +++ platform/node/test/js/map.test.js | 8 +++++++ src/mbgl/style/sources/raster_source.cpp | 3 ++- src/mbgl/style/sources/vector_source.cpp | 3 ++- 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp index 0cc93d7e95..7dbccfcf41 100644 --- a/platform/node/src/node_map.cpp +++ b/platform/node/src/node_map.cpp @@ -47,6 +47,7 @@ struct NodeMap::RenderOptions { }; Nan::Persistent NodeMap::constructor; +Nan::Persistent NodeMap::parseError; static const char* releasedMessage() { return "Map resources have already been released"; @@ -57,6 +58,20 @@ void NodeMapObserver::onDidFailLoadingMap(std::exception_ptr error) { } void NodeMap::Init(v8::Local target) { + // Define a custom error class for parse errors + auto script = Nan::New(Nan::New(R"JS( +class ParseError extends Error { + constructor(...params) { + super(...params); + if (Error.captureStackTrace) { + Error.captureStackTrace(this, ParseError); + } + } +} +ParseError)JS").ToLocalChecked()).ToLocalChecked(); + parseError.Reset(Nan::To(Nan::RunScript(script).ToLocalChecked()).ToLocalChecked()); + Nan::Set(target, Nan::New("ParseError").ToLocalChecked(), Nan::New(parseError)); + v8::Local tpl = Nan::New(New); tpl->SetClassName(Nan::New("Map").ToLocalChecked()); @@ -216,6 +231,8 @@ void NodeMap::Load(const Nan::FunctionCallbackInfo& info) { try { nodeMap->map->getStyle().loadJSON(style); + } catch (const mbgl::util::StyleParseException& ex) { + return Nan::ThrowError(ParseError(ex.what())); } catch (const std::exception &ex) { return Nan::ThrowError(ex.what()); } @@ -408,9 +425,11 @@ void NodeMap::Render(const Nan::FunctionCallbackInfo& info) { nodeMap->req = std::make_unique(Nan::To(info[1]).ToLocalChecked()); nodeMap->startRender(std::move(options)); - } catch (mbgl::style::conversion::Error& err) { + } catch (const mbgl::style::conversion::Error& err) { return Nan::ThrowTypeError(err.message.c_str()); - } catch (mbgl::util::Exception &ex) { + } catch (const mbgl::util::StyleParseException& ex) { + return Nan::ThrowError(ParseError(ex.what())); + } catch (const mbgl::util::Exception &ex) { return Nan::ThrowError(ex.what()); } @@ -459,6 +478,11 @@ void NodeMap::startRender(NodeMap::RenderOptions options) { uv_ref(reinterpret_cast(async)); } +v8::Local NodeMap::ParseError(const char* msg) { + v8::Local argv[] = { Nan::New(msg).ToLocalChecked() }; + return Nan::CallAsConstructor(Nan::New(parseError), 1, argv).ToLocalChecked(); +} + void NodeMap::renderFinished() { assert(req); @@ -480,16 +504,19 @@ void NodeMap::renderFinished() { v8::Local target = Nan::New(); if (error) { - std::string errorMessage; + v8::Local err; try { std::rethrow_exception(error); + assert(false); + } catch (const mbgl::util::StyleParseException& ex) { + err = ParseError(ex.what()); } catch (const std::exception& ex) { - errorMessage = ex.what(); + err = Nan::Error(ex.what()); } v8::Local argv[] = { - Nan::Error(errorMessage.c_str()) + err }; // This must be empty to be prepared for the next render call. diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp index 19df095481..52fc1ef659 100644 --- a/platform/node/src/node_map.hpp +++ b/platform/node/src/node_map.hpp @@ -37,6 +37,7 @@ public: ~NodeMap(); static Nan::Persistent constructor; + static Nan::Persistent parseError; static void Init(v8::Local); @@ -67,6 +68,8 @@ public: static void DumpDebugLogs(const Nan::FunctionCallbackInfo&); static void QueryRenderedFeatures(const Nan::FunctionCallbackInfo&); + static v8::Local ParseError(const char* msg); + void startRender(RenderOptions options); void renderFinished(); diff --git a/platform/node/test/js/map.test.js b/platform/node/test/js/map.test.js index 2223e54aaa..4a71d72046 100644 --- a/platform/node/test/js/map.test.js +++ b/platform/node/test/js/map.test.js @@ -323,6 +323,10 @@ test('Map', function(t) { map.load('""'); }, /Failed to parse style: style must be an object/); + t.throws(function() { + map.load('""'); + }, mbgl.ParseError); + map.release(); t.end(); }); @@ -339,6 +343,10 @@ test('Map', function(t) { t.end(); }); + t.throws(function() { + map.load('invalid'); + }, mbgl.ParseError); + t.throws(function() { map.load('invalid'); }, /Failed to parse style: 0 - Invalid value./); diff --git a/src/mbgl/style/sources/raster_source.cpp b/src/mbgl/style/sources/raster_source.cpp index 53f29d660b..c2f96dbd55 100644 --- a/src/mbgl/style/sources/raster_source.cpp +++ b/src/mbgl/style/sources/raster_source.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace mbgl { namespace style { @@ -59,7 +60,7 @@ void RasterSource::loadDescription(FileSource& fileSource) { conversion::Error error; optional tileset = conversion::convertJSON(*res.data, error); if (!tileset) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(error.message))); + observer->onSourceError(*this, std::make_exception_ptr(util::StyleParseException(error.message))); return; } diff --git a/src/mbgl/style/sources/vector_source.cpp b/src/mbgl/style/sources/vector_source.cpp index ccdd453c75..6cede8fae9 100644 --- a/src/mbgl/style/sources/vector_source.cpp +++ b/src/mbgl/style/sources/vector_source.cpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace mbgl { namespace style { @@ -56,7 +57,7 @@ void VectorSource::loadDescription(FileSource& fileSource) { conversion::Error error; optional tileset = conversion::convertJSON(*res.data, error); if (!tileset) { - observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error(error.message))); + observer->onSourceError(*this, std::make_exception_ptr(util::StyleParseException(error.message))); return; } -- cgit v1.2.1