summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2022-06-15 20:05:36 -0400
committerNat Goodspeed <nat@lindenlab.com>2022-06-15 20:05:36 -0400
commitc7408f842e64e468edae93c219056c82ff077e0e (patch)
tree1a7c57cc856a4aefeaed633d027771f5b95f563f
parenta4ff9caf69cb8fbbf3e5d40a258ec99a070b0f94 (diff)
parent4e7b4bab79be8cf2de9af242e5cd23347fba8bb2 (diff)
DRTVWR-564: Merge branch 'pick-eventapi' into lazy-eventpump
to pick up generalized LLEventAPI add() methods and softer error handling.
-rw-r--r--indra/llcommon/lleventapi.h13
-rw-r--r--indra/llcommon/lleventdispatcher.cpp127
-rw-r--r--indra/llcommon/lleventdispatcher.h114
-rw-r--r--indra/llcommon/tests/lleventdispatcher_test.cpp62
4 files changed, 209 insertions, 107 deletions
diff --git a/indra/llcommon/lleventapi.h b/indra/llcommon/lleventapi.h
index 5991fe8fd5..ed62fa064a 100644
--- a/indra/llcommon/lleventapi.h
+++ b/indra/llcommon/lleventapi.h
@@ -65,19 +65,6 @@ public:
std::string getDesc() const { return mDesc; }
/**
- * Publish only selected add() methods from LLEventDispatcher.
- * Every LLEventAPI add() @em must have a description string.
- */
- template <typename CALLABLE>
- void add(const std::string& name,
- const std::string& desc,
- CALLABLE callable,
- const LLSD& required=LLSD())
- {
- LLEventDispatcher::add(name, desc, callable, required);
- }
-
- /**
* Instantiate a Response object in any LLEventAPI subclass method that
* wants to guarantee a reply (if requested) will be sent on exit from the
* method. The reply will be sent if request.has(@a replyKey), default
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..1b3e834aeb 100644
--- a/indra/llcommon/lleventdispatcher.h
+++ b/indra/llcommon/lleventdispatcher.h
@@ -61,7 +61,6 @@ static const auto& nil(nil_);
#include <boost/function.hpp>
#include <boost/bind.hpp>
#include <boost/iterator/transform_iterator.hpp>
-#include <boost/utility/enable_if.hpp>
#include <boost/function_types/is_nonmember_callable_builtin.hpp>
#include <boost/function_types/parameter_types.hpp>
#include <boost/function_types/function_arity.hpp>
@@ -167,10 +166,11 @@ public:
* converted to the corresponding parameter type using LLSDParam.
*/
template<typename Function>
- typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin<Function>
- >::type add(const std::string& name,
- const std::string& desc,
- Function f);
+ typename std::enable_if<
+ boost::function_types::is_nonmember_callable_builtin<Function>::value
+ >::type add(const std::string& name,
+ const std::string& desc,
+ Function f);
/**
* Register a nonstatic class method with arbitrary parameters.
@@ -190,11 +190,13 @@ public:
* converted to the corresponding parameter type using LLSDParam.
*/
template<typename Method, typename InstanceGetter>
- typename boost::enable_if< boost::function_types::is_member_function_pointer<Method>
- >::type add(const std::string& name,
- const std::string& desc,
- Method f,
- const InstanceGetter& getter);
+ typename std::enable_if<
+ boost::function_types::is_member_function_pointer<Method>::value &&
+ ! std::is_convertible<InstanceGetter, LLSD>::value
+ >::type add(const std::string& name,
+ const std::string& desc,
+ Method f,
+ const InstanceGetter& getter);
/**
* Register a free function with arbitrary parameters. (This also works
@@ -212,12 +214,13 @@ public:
* to the corresponding parameter type using LLSDParam.
*/
template<typename Function>
- typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin<Function>
- >::type add(const std::string& name,
- const std::string& desc,
- Function f,
- const LLSD& params,
- const LLSD& defaults=LLSD());
+ typename std::enable_if<
+ boost::function_types::is_nonmember_callable_builtin<Function>::value
+ >::type add(const std::string& name,
+ const std::string& desc,
+ Function f,
+ const LLSD& params,
+ const LLSD& defaults=LLSD());
/**
* Register a nonstatic class method with arbitrary parameters.
@@ -241,41 +244,43 @@ public:
* to the corresponding parameter type using LLSDParam.
*/
template<typename Method, typename InstanceGetter>
- typename boost::enable_if< boost::function_types::is_member_function_pointer<Method>
- >::type add(const std::string& name,
- const std::string& desc,
- Method f,
- const InstanceGetter& getter,
- const LLSD& params,
- const LLSD& defaults=LLSD());
+ typename std::enable_if<
+ boost::function_types::is_member_function_pointer<Method>::value &&
+ ! std::is_convertible<InstanceGetter, LLSD>::value
+ >::type add(const std::string& name,
+ const std::string& desc,
+ Method f,
+ const InstanceGetter& getter,
+ const LLSD& params,
+ const LLSD& defaults=LLSD());
//@}
/// 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 +345,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;
@@ -427,7 +439,25 @@ struct LLEventDispatcher::invoker
// Instead of grabbing the first item from argsrc and making an
// LLSDParam of it, call getter() and pass that as the instance param.
invoker<Function, next_iter_type, To>::apply
- ( func, argsrc, boost::fusion::push_back(boost::fusion::nil(), boost::ref(getter())));
+ ( func, argsrc, boost::fusion::push_back(boost::fusion::nil(), bindable(getter())));
+ }
+
+ template <typename T>
+ static inline
+ auto bindable(T&& value,
+ typename std::enable_if<std::is_pointer<T>::value, bool>::type=true)
+ {
+ // if passed a pointer, just return that pointer
+ return std::forward<T>(value);
+ }
+
+ template <typename T>
+ static inline
+ auto bindable(T&& value,
+ typename std::enable_if<! std::is_pointer<T>::value, bool>::type=true)
+ {
+ // if passed a reference, wrap it for binding
+ return std::ref(std::forward<T>(value));
}
};
@@ -447,7 +477,7 @@ struct LLEventDispatcher::invoker<Function,To,To>
};
template<typename Function>
-typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin<Function> >::type
+typename std::enable_if< boost::function_types::is_nonmember_callable_builtin<Function>::value >::type
LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f)
{
// Construct an invoker_function, a callable accepting const args_source&.
@@ -458,7 +488,10 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio
}
template<typename Method, typename InstanceGetter>
-typename boost::enable_if< boost::function_types::is_member_function_pointer<Method> >::type
+typename std::enable_if<
+ boost::function_types::is_member_function_pointer<Method>::value &&
+ ! std::is_convertible<InstanceGetter, LLSD>::value
+>::type
LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f,
const InstanceGetter& getter)
{
@@ -469,7 +502,7 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Method
}
template<typename Function>
-typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin<Function> >::type
+typename std::enable_if< boost::function_types::is_nonmember_callable_builtin<Function>::value >::type
LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f,
const LLSD& params, const LLSD& defaults)
{
@@ -478,7 +511,10 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio
}
template<typename Method, typename InstanceGetter>
-typename boost::enable_if< boost::function_types::is_member_function_pointer<Method> >::type
+typename std::enable_if<
+ boost::function_types::is_member_function_pointer<Method>::value &&
+ ! std::is_convertible<InstanceGetter, LLSD>::value
+>::type
LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f,
const InstanceGetter& getter,
const LLSD& params, const LLSD& defaults)
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");
}
}
}