Improve handling of errors in pubsub code

include/pubsub.hrl

@@ -26,7 +26,7 @@
 -define(MAXITEMS, 10).
 
 %% this is currently a hard limit.
-%% Would be nice to have it configurable. 
+%% Would be nice to have it configurable.
 -define(MAX_PAYLOAD_SIZE, 250000).
 
 %% -------------------------------
@@ -84,7 +84,7 @@
      Value::binary() | [binary()] | boolean()
 }).
 
--type(subOptions() :: [mod_pubsub:subOption(),...]).
+-type(subOptions() :: [mod_pubsub:subOption()]).
 
 -type(pubOption() ::
     {Option::binary(),

src/gen_pubsub_node.erl

@@ -142,7 +142,7 @@
 -callback set_affiliation(NodeIdx :: nodeIdx(),
 	Owner :: jid(),
 	Affiliation :: affiliation()) ->
-    ok |
+    {result, ok} |
     {error, stanza_error()}.
 
 -callback get_node_subscriptions(NodeIdx :: nodeIdx()) ->
@@ -208,10 +208,10 @@
 -callback get_item_name(Host :: host(),
 	ServerHost :: binary(),
 	Node :: nodeId()) ->
-    itemId().
+    {result, itemId()}.
 
 -callback node_to_path(Node :: nodeId()) ->
-    [nodeId()].
+    {result, [nodeId()]}.
 
 -callback path_to_node(Node :: [nodeId()]) ->
-    nodeId().
+    {result, nodeId()}.

src/mod_caps.erl

@@ -106,7 +106,7 @@ list_features(C2SState) ->
     Rs = maps:get(caps_resources, C2SState, gb_trees:empty()),
     gb_trees:to_list(Rs).
 
--spec get_user_caps(jid(), ejabberd_c2s:state()) -> {ok, caps()} | error.
+-spec get_user_caps(jid() | ljid(), ejabberd_c2s:state()) -> {ok, caps()} | error.
 get_user_caps(JID, C2SState) ->
     Rs = maps:get(caps_resources, C2SState, gb_trees:empty()),
     LJID = jid:tolower(JID),

src/node_flat.erl

@@ -535,8 +535,8 @@ set_affiliation(Nidx, Owner, Affiliation) ->
     GenKey = jid:remove_resource(SubKey),
     GenState = get_state(Nidx, GenKey),
     case {Affiliation, GenState#pubsub_state.subscriptions} of
-	{none, []} -> del_state(GenState);
-	_ -> set_state(GenState#pubsub_state{affiliation = Affiliation})
+	{none, []} -> {result, del_state(GenState)};
+	_ -> {result, set_state(GenState#pubsub_state{affiliation = Affiliation})}
     end.
 
 %% @doc <p>Return the current subscriptions for the given user</p>
@@ -616,7 +616,7 @@ set_subscriptions(Nidx, Owner, Subscription, SubId) ->
 
 replace_subscription(NewSub, SubState) ->
     NewSubs = replace_subscription(NewSub, SubState#pubsub_state.subscriptions, []),
-    set_state(SubState#pubsub_state{subscriptions = NewSubs}).
+    {result, set_state(SubState#pubsub_state{subscriptions = NewSubs})}.
 
 replace_subscription(_, [], Acc) -> Acc;
 replace_subscription({Sub, SubId}, [{_, SubId} | T], Acc) ->
@@ -627,7 +627,7 @@ new_subscription(_Nidx, _Owner, Sub, SubState) ->
     SubId = pubsub_subscription:make_subid(),
     Subs = SubState#pubsub_state.subscriptions,
     set_state(SubState#pubsub_state{subscriptions = [{Sub, SubId} | Subs]}),
-    {Sub, SubId}.
+    {result, {Sub, SubId}}.
 
 unsub_with_subid(SubState, SubId) ->
     %%pubsub_subscription:delete_subscription(SubState#pubsub_state.stateid, Nidx, SubId),
@@ -635,8 +635,8 @@ unsub_with_subid(SubState, SubId) ->
 	    || {S, Sid} <- SubState#pubsub_state.subscriptions,
 		SubId =/= Sid],
     case {NewSubs, SubState#pubsub_state.affiliation} of
-	{[], none} -> del_state(SubState);
-	_ -> set_state(SubState#pubsub_state{subscriptions = NewSubs})
+	{[], none} -> {result, del_state(SubState)};
+	_ -> {result, set_state(SubState#pubsub_state{subscriptions = NewSubs})}
     end.
 
 %% @doc <p>Returns a list of Owner's nodes on Host with pending
@@ -884,22 +884,23 @@ del_orphan_items(Nidx) ->
     end.
 
 get_item_name(_Host, _Node, Id) ->
-    Id.
+    {result, Id}.
 
 %% @doc <p>Return the path of the node. In flat it's just node id.</p>
 node_to_path(Node) ->
-    [(Node)].
+    {result, [Node]}.
 
 path_to_node(Path) ->
-    case Path of
-	% default slot
-	[Node] -> iolist_to_binary(Node);
-	% handle old possible entries, used when migrating database content to new format
-	[Node | _] when is_binary(Node) ->
-	    iolist_to_binary(str:join([<<"">> | Path], <<"/">>));
-	% default case (used by PEP for example)
-	_ -> iolist_to_binary(Path)
-    end.
+    {result,
+     case Path of
+	 %% default slot
+	 [Node] -> iolist_to_binary(Node);
+	 %% handle old possible entries, used when migrating database content to new format
+	 [Node | _] when is_binary(Node) ->
+	     iolist_to_binary(str:join([<<"">> | Path], <<"/">>));
+	 %% default case (used by PEP for example)
+	 _ -> iolist_to_binary(Path)
+     end}.
 
 can_fetch_item(owner, _) -> true;
 can_fetch_item(member, _) -> true;

src/node_flat_sql.erl

@@ -377,8 +377,8 @@ set_affiliation(Nidx, Owner, Affiliation) ->
     GenKey = jid:remove_resource(SubKey),
     {_, Subscriptions} = select_affiliation_subscriptions(Nidx, GenKey),
     case {Affiliation, Subscriptions} of
-	{none, []} -> del_state(Nidx, GenKey);
-	_ -> update_affiliation(Nidx, GenKey, Affiliation)
+	{none, []} -> {result, del_state(Nidx, GenKey)};
+	_ -> {result, update_affiliation(Nidx, GenKey, Affiliation)}
     end.
 
 get_entity_subscriptions(Host, Owner) ->
@@ -522,7 +522,7 @@ set_subscriptions(Nidx, Owner, Subscription, SubId) ->
 
 replace_subscription(NewSub, SubState) ->
     NewSubs = replace_subscription(NewSub, SubState#pubsub_state.subscriptions, []),
-    set_state(SubState#pubsub_state{subscriptions = NewSubs}).
+    {result, set_state(SubState#pubsub_state{subscriptions = NewSubs})}.
 
 replace_subscription(_, [], Acc) -> Acc;
 replace_subscription({Sub, SubId}, [{_, SubId} | T], Acc) ->
@@ -533,7 +533,7 @@ new_subscription(_Nidx, _Owner, Subscription, SubState) ->
     SubId = pubsub_subscription_sql:make_subid(),
     Subscriptions = [{Subscription, SubId} | SubState#pubsub_state.subscriptions],
     set_state(SubState#pubsub_state{subscriptions = Subscriptions}),
-    {Subscription, SubId}.
+    {result, {Subscription, SubId}}.
 
 unsub_with_subid(Nidx, SubId, SubState) ->
     %%pubsub_subscription_sql:unsubscribe_node(SubState#pubsub_state.stateid, Nidx, SubId),
@@ -541,8 +541,8 @@ unsub_with_subid(Nidx, SubId, SubState) ->
 	    || {S, Sid} <- SubState#pubsub_state.subscriptions,
 		SubId =/= Sid],
     case {NewSubs, SubState#pubsub_state.affiliation} of
-	{[], none} -> del_state(Nidx, element(1, SubState#pubsub_state.stateid));
-	_ -> set_state(SubState#pubsub_state{subscriptions = NewSubs})
+	{[], none} -> {result, del_state(Nidx, element(1, SubState#pubsub_state.stateid))};
+	_ -> {result, set_state(SubState#pubsub_state{subscriptions = NewSubs})}
     end.
 
 get_pending_nodes(Host, Owner) ->
@@ -825,7 +825,7 @@ del_items(Nidx, ItemIds) ->
 	    I, <<") and nodeid='">>, SNidx, <<"';">>]).
 
 get_item_name(_Host, _Node, Id) ->
-    Id.
+    {result, Id}.
 
 node_to_path(Node) ->
     node_flat:node_to_path(Node).

src/nodetree_tree.erl

@@ -139,11 +139,11 @@ get_subnodes_tree(Host, Node) ->
 	Rec ->
 	    BasePlugin = misc:binary_to_atom(<<"node_",
 			(Rec#pubsub_node.type)/binary>>),
-	    BasePath = BasePlugin:node_to_path(Node),
+	    {result, BasePath} = BasePlugin:node_to_path(Node),
 	    mnesia:foldl(fun (#pubsub_node{nodeid = {H, N}} = R, Acc) ->
 			Plugin = misc:binary_to_atom(<<"node_",
 				    (R#pubsub_node.type)/binary>>),
-			Path = Plugin:node_to_path(N),
+			{result, Path} = Plugin:node_to_path(N),
 			case lists:prefix(BasePath, Path) and (H == Host) of
 			    true -> [R | Acc];
 			    false -> Acc