diff options
| -rw-r--r-- | indra/llcommon/lleventdispatcher.cpp | 127 | ||||
| -rw-r--r-- | indra/llcommon/lleventdispatcher.h | 29 | ||||
| -rw-r--r-- | indra/llcommon/tests/lleventdispatcher_test.cpp | 62 |
3 files changed, 152 insertions, 66 deletions
diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index 5b6d4efbe9..742d6cf51f 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -40,15 +40,24 @@ // other Linden headers #include "llevents.h" #include "llerror.h" +#include "llexception.h" #include "llsdutil.h" #include "stringize.h" #include <memory> // std::auto_ptr /***************************************************************************** +* DispatchError +*****************************************************************************/ +struct DispatchError: public LLException +{ + DispatchError(const std::string& what): LLException(what) {} +}; + +/***************************************************************************** * LLSDArgsSource *****************************************************************************/ /** - * Store an LLSD array, producing its elements one at a time. Die with LL_ERRS + * Store an LLSD array, producing its elements one at a time. It is an error * if the consumer requests more elements than the array contains. */ class LL_COMMON_API LLSDArgsSource @@ -74,8 +83,7 @@ LLSDArgsSource::LLSDArgsSource(const std::string function, const LLSD& args): { if (! (_args.isUndefined() || _args.isArray())) { - LL_ERRS("LLSDArgsSource") << _function << " needs an args array instead of " - << _args << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " needs an args array instead of ", _args))); } } @@ -88,8 +96,8 @@ LLSD LLSDArgsSource::next() { if (_index >= _args.size()) { - LL_ERRS("LLSDArgsSource") << _function << " requires more arguments than the " - << _args.size() << " provided: " << _args << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " requires more arguments than the ", + _args.size(), " provided: ", _args))); } return _args[_index++]; } @@ -163,7 +171,8 @@ public: /// default values LLSDArgsMapper(const std::string& function, const LLSD& names, const LLSD& defaults); - /// Given arguments map, return LLSD::Array of parameter values, or LL_ERRS. + /// Given arguments map, return LLSD::Array of parameter values, or + /// trigger error. LLSD map(const LLSD& argsmap) const; private: @@ -195,7 +204,7 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, { if (! (_names.isUndefined() || _names.isArray())) { - LL_ERRS("LLSDArgsMapper") << function << " names must be an array, not " << names << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " names must be an array, not ", names))); } LLSD::Integer nparams(_names.size()); // From _names generate _indexes. @@ -218,8 +227,8 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, // defaults is a (possibly empty) array. Right-align it with names. if (ndefaults > nparams) { - LL_ERRS("LLSDArgsMapper") << function << " names array " << names - << " shorter than defaults array " << defaults << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " names array ", names, + " shorter than defaults array ", defaults))); } // Offset by which we slide defaults array right to right-align with @@ -256,14 +265,14 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, } if (bogus.size()) { - LL_ERRS("LLSDArgsMapper") << function << " defaults specified for nonexistent params " - << formatlist(bogus) << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " defaults specified for nonexistent params ", + formatlist(bogus)))); } } else { - LL_ERRS("LLSDArgsMapper") << function << " defaults must be a map or an array, not " - << defaults << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " defaults must be a map or an array, not ", + defaults))); } } @@ -271,8 +280,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const { if (! (argsmap.isUndefined() || argsmap.isMap() || argsmap.isArray())) { - LL_ERRS("LLSDArgsMapper") << _function << " map() needs a map or array, not " - << argsmap << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " map() needs a map or array, not ", + argsmap))); } // Initialize the args array. Indexing a non-const LLSD array grows it // to appropriate size, but we don't want to resize this one on each @@ -369,8 +378,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const // by argsmap, that's a problem. if (unfilled.size()) { - LL_ERRS("LLSDArgsMapper") << _function << " missing required arguments " - << formatlist(unfilled) << " from " << argsmap << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " missing required arguments ", + formatlist(unfilled), " from ", argsmap))); } // done @@ -420,7 +429,7 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE std::string mismatch(llsd_matches(mRequired, event)); if (! mismatch.empty()) { - LL_ERRS("LLEventDispatcher") << desc << ": bad request: " << mismatch << LL_ENDL; + LLTHROW(DispatchError(stringize(desc, ": bad request: ", mismatch))); } // Event syntax looks good, go for it! mFunc(event); @@ -596,48 +605,90 @@ bool LLEventDispatcher::remove(const std::string& name) return true; } -/// Call a registered callable with an explicitly-specified name. If no -/// such callable exists, die with LL_ERRS. +/// Call a registered callable with an explicitly-specified name. It is an +/// error if no such callable exists. void LLEventDispatcher::operator()(const std::string& name, const LLSD& event) const { - if (! try_call(name, event)) + std::string error{ try_call_log(std::string(), name, event) }; + if (! error.empty()) { - LL_ERRS("LLEventDispatcher") << "LLEventDispatcher(" << mDesc << "): '" << name - << "' not found" << LL_ENDL; + callFail(event, error); } } -/// Extract the @a key value from the incoming @a event, and call the -/// callable whose name is specified by that map @a key. If no such -/// callable exists, die with LL_ERRS. +/// Extract the @a key value from the incoming @a event, and call the callable +/// whose name is specified by that map @a key. It is an error if no such +/// callable exists. void LLEventDispatcher::operator()(const LLSD& event) const { - // This could/should be implemented in terms of the two-arg overload. - // However -- we can produce a more informative error message. - std::string name(event[mKey]); - if (! try_call(name, event)) + std::string error{ try_call_log(mKey, event[mKey], event) }; + if (! error.empty()) + { + callFail(event, error); + } +} + +void LLEventDispatcher::callFail(const LLSD& event, const std::string& msg) const +{ + static LLSD::String key{ "reply" }; + if (event.has(key)) { - LL_ERRS("LLEventDispatcher") << "LLEventDispatcher(" << mDesc << "): bad " << mKey - << " value '" << name << "'" << LL_ENDL; + // Oh good, the incoming event specifies a reply pump -- pass back a + // response that includes an "error" key with the message. + sendReply(llsd::map("error", msg), event, key); } } bool LLEventDispatcher::try_call(const LLSD& event) const { - return try_call(event[mKey], event); + return try_call_log(mKey, event[mKey], event).empty(); } bool LLEventDispatcher::try_call(const std::string& name, const LLSD& event) const { + return try_call_log(std::string(), name, event).empty(); +} + +std::string LLEventDispatcher::try_call_log(const std::string& key, const std::string& name, + const LLSD& event) const +{ + std::string error{ try_call(key, name, event) }; + if (! error.empty()) + { + LL_WARNS("LLEventDispatcher") << error << LL_ENDL; + } + return error; +} + +// This internal method returns empty string if the call succeeded, else +// non-empty error message. +std::string LLEventDispatcher::try_call(const std::string& key, const std::string& name, + const LLSD& event) const +{ DispatchMap::const_iterator found = mDispatch.find(name); if (found == mDispatch.end()) { - return false; + if (key.empty()) + { + return stringize("LLEventDispatcher(", mDesc, "): '", name, "' not found"); + } + else + { + return stringize("LLEventDispatcher(", mDesc, "): bad ", key, " value '", name, "'"); + } + } + + try + { + // Found the name, so it's plausible to even attempt the call. + found->second->call(stringize("LLEventDispatcher(", mDesc, ") calling '", name, "'"), + event); + } + catch (const DispatchError& err) + { + return err.what(); } - // Found the name, so it's plausible to even attempt the call. - found->second->call(STRINGIZE("LLEventDispatcher(" << mDesc << ") calling '" << name << "'"), - event); - return true; // tell caller we were able to call + return {}; // tell caller we were able to call } LLSD LLEventDispatcher::getMetadata(const std::string& name) const diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 9e1244ef5b..b78c77f8b3 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -254,28 +254,28 @@ public: /// Unregister a callable bool remove(const std::string& name); - /// Call a registered callable with an explicitly-specified name. If no - /// such callable exists, die with LL_ERRS. If the @a event fails to match - /// the @a required prototype specified at add() time, die with LL_ERRS. + /// Call a registered callable with an explicitly-specified name. It is an + /// error if no such callable exists. It is an error if the @a event fails + /// to match the @a required prototype specified at add() time. void operator()(const std::string& name, const LLSD& event) const; /// Call a registered callable with an explicitly-specified name and /// return <tt>true</tt>. If no such callable exists, return - /// <tt>false</tt>. If the @a event fails to match the @a required - /// prototype specified at add() time, die with LL_ERRS. + /// <tt>false</tt>. It is an error if the @a event fails to match the @a + /// required prototype specified at add() time. bool try_call(const std::string& name, const LLSD& event) const; /// Extract the @a key value from the incoming @a event, and call the - /// callable whose name is specified by that map @a key. If no such - /// callable exists, die with LL_ERRS. If the @a event fails to match the - /// @a required prototype specified at add() time, die with LL_ERRS. + /// callable whose name is specified by that map @a key. It is an error if + /// no such callable exists. It is an error if the @a event fails to match + /// the @a required prototype specified at add() time. void operator()(const LLSD& event) const; /// Extract the @a key value from the incoming @a event, call the callable /// whose name is specified by that map @a key and return <tt>true</tt>. - /// If no such callable exists, return <tt>false</tt>. If the @a event - /// fails to match the @a required prototype specified at add() time, die - /// with LL_ERRS. + /// If no such callable exists, return <tt>false</tt>. It is an error if + /// the @a event fails to match the @a required prototype specified at + /// add() time. bool try_call(const LLSD& event) const; /// @name Iterate over defined names @@ -340,6 +340,13 @@ private: } } void addFail(const std::string& name, const std::string& classname) const; + std::string try_call_log(const std::string& key, const std::string& name, + const LLSD& event) const; + std::string try_call(const std::string& key, const std::string& name, + const LLSD& event) const; + // Implement "it is an error" semantics for attempted call operations: if + // the incoming event includes a "reply" key, log and send an error reply. + void callFail(const LLSD& event, const std::string& msg) const; std::string mDesc, mKey; DispatchMap mDispatch; diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 9da1ecfd67..82a0ddf61b 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -20,6 +20,7 @@ #include "../test/lltut.h" #include "llsd.h" #include "llsdutil.h" +#include "llevents.h" #include "stringize.h" #include "tests/wrapllerrs.h" #include "../test/catch_and_store_what_in.h" @@ -644,12 +645,45 @@ namespace tut outer.find(inner) != std::string::npos); } - void call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) + std::string call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - std::string threw = catch_what<std::runtime_error>([this, &func, &args](){ - work(func, args); - }); - ensure_has(threw, exc_frag); + // This method was written when LLEventDispatcher responded to + // name or argument errors with LL_ERRS, hence the name: we used + // to have to intercept LL_ERRS by making it throw. Now we set up + // to catch an error response instead. But -- for that we need to + // be able to sneak a "reply" key into args, which must be a Map. + if (! (args.isUndefined() or args.isMap())) + fail(stringize("can't test call_exc() with ", args)); + LLEventStream replypump("reply"); + LLSD reply; + LLTempBoundListener bound{ + replypump.listen( + "listener", + [&reply](const LLSD& event) + { + reply = event; + return false; + }) }; + LLSD modargs{ args }; + modargs["reply"] = replypump.getName(); + if (func.empty()) + { + work(modargs); + } + else + { + work(func, modargs); + } + ensure("no error response", reply.has("error")); + ensure_has(reply["error"], exc_frag); + return reply["error"]; + } + + void call_logerr(const std::string& func, const LLSD& args, const std::string& frag) + { + CaptureLog capture; + work(func, args); + capture.messageWith(frag); } LLSD getMetadata(const std::string& name) @@ -1031,13 +1065,7 @@ namespace tut { set_test_name("call with bad name"); call_exc("freek", LLSD(), "not found"); - // We don't have a comparable helper function for the one-arg - // operator() method, and it's not worth building one just for this - // case. Write it out. - std::string threw = catch_what<std::runtime_error>([this](){ - work(LLSDMap("op", "freek")); - }); - ensure_has(threw, "bad"); + std::string threw = call_exc("", LLSDMap("op", "freek"), "bad"); ensure_has(threw, "op"); ensure_has(threw, "freek"); } @@ -1087,7 +1115,7 @@ namespace tut ensure_equals("answer mismatch", tr.llsd, answer); // Should NOT be able to pass 'answer' to Callables registered // with 'required'. - call_exc(tr.name_req, answer, "bad request"); + call_logerr(tr.name_req, answer, "bad request"); // But SHOULD be able to pass 'matching' to Callables registered // with 'required'. work(tr.name_req, matching); @@ -1107,11 +1135,11 @@ namespace tut // args. We should only need to engage it for one map-style // registration and one array-style registration. std::string array_exc("needs an args array"); - call_exc("free0_array", 17, array_exc); - call_exc("free0_array", LLSDMap("pi", 3.14), array_exc); + call_logerr("free0_array", 17, array_exc); + call_logerr("free0_array", LLSDMap("pi", 3.14), array_exc); std::string map_exc("needs a map"); - call_exc("free0_map", 17, map_exc); + call_logerr("free0_map", 17, map_exc); // Passing an array to a map-style function works now! No longer an // error case! // call_exc("free0_map", LLSDArray("a")("b"), map_exc); @@ -1158,7 +1186,7 @@ namespace tut { foreach(const llsd::MapEntry& e, inMap(funcsab)) { - call_exc(e.second, tooshort, "requires more arguments"); + call_logerr(e.second, tooshort, "requires more arguments"); } } } |