views:

174

answers:

3

Basically, my question is short and sweet: Is the following a bad idea (encapsulating and rethrowing ex.InnerException instead of ex)

(There's a similar question here, but not quite... I want to reencapsulate the InnerException, so the stack trace is preserved without reflecting on internals)

public abstract class RpcProvider
{
    public virtual object CallMethod(string methodName, params object[] parameters)
    {
        MethodInfo mi = this.GetType().GetMethod(methodName);

        if (mi == null || mi.GetCustomAttributes(typeof(RpcCallAttribute), true).Length == 0)
        {
            throw new NotImplementedException("This method is not provided by this RPC provider.");
        }
        else
        {
            try
            {
                return mi.Invoke(this, parameters);
            }
            catch (TargetInvocationException ex)
            {
                throw new RpcException("There was an error in the RPC call. See the InnerException for details.", ex.InnerException);
            }
        }
    }
}

The stacktrace below seems intact and fine (well, it's sans the internals of how reflection invokes the method), so is there an issue with this at all? For the stacktrace below to make sense, my inheritance hierachy is:

 -Oxide.Net.Rpc.RpcProvider
 |-Oxide.Net.Rpc.XmlRpc
  |-StartMenuSorter.DesktopMasters

(sanitised to protect the innocent, ie. me)

at Oxide.Net.Rpc.XmlRpc.DoRequest(Uri rpcConnectionUri, IXPathNavigable request, String userAgent) in \Projects\OxideLib\Oxide.Net\Rpc\XmlRpc.cs:line 243
at StartMenuSorter.DesktopMasters.GetIconInformation(IEnumerable`1 icons) in \Projects\StartMenuSorter\StartMenuSorter\DesktopMasters.cs:line 17
at Oxide.Net.Rpc.RpcProvider.CallMethod(String methodName, Object[] parameters) in \Projects\OxideLib\Oxide.Net\Rpc\RpcProvider.cs:line 52
at StartMenuSorter.Program.Main() in \Projects\StartMenuSorter\StartMenuSorter\Program.cs:line 36
at System.AppDomain._nExecuteAssembly(Assembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
A: 

I've done this before; it worked quite well for me, and it was essentially the same form as you used in your example.

Robert Harvey
+1  A: 

I see no problem in this. You're basically taking one type of exception and translating it into a more meaningful exception. I believe this is a good approach in this case.

Reed Copsey
+1  A: 

Looks completely reasonable: the stacktrace includes details of where the RpcProvider is calling the method, and hides gory and unnecessary reflection gubbins, so you're fine.

As with all error handling code, the ultimate consumer will be other developers, so it would be good to ask "do I have enough detail to debug this myself if something goes wrong?"

As there's not so much noise in the way as if the outer exception were thrown the technique does its job well.

Jeremy McGee