Issue? / workaround inside public T MakeRequest<T> !? about bitcoinlib HOT 3 CLOSED

It all has to do with what you are trying to achieve. The way it is currently designed, your app will not get the rpcResponse.Error details, however an exception will be raised (try adding a FirstChanceException handler to your AppDomain: AppDomain.CurrentDomain.FirstChanceException += FirstChanceExceptionHandler;), so as long as you perform your crucial RPC calls in a Try/Catch block the current implementation will serve as a production-level framework for your app. If on the other hand you believe that the rpcResponse.Error's content is crucial for your app then you can throw an exception (or even execute any custom exception handling code) as-it-happens:

    public T MakeRequest<T>(RpcMethods rpcMethod, params object[] parameters)
    {
        JsonRpcResponse<T> rpcResponse = MakeRpcRequest<T>(new JsonRpcRequest(1, rpcMethod.ToString(), parameters), 0);

        if (!String.IsNullOrWhiteSpace(rpcResponse.Error))
        {
            throw new Exception(rpcResponse.Error);
        }

        return rpcResponse.Result;
    }

For RPC methods that return String or Object, this will suffice:
return String.IsNullOrEmpty(rpcResponse.Error) ? rpcResponse.Result : (T)(object)rpcResponse.Error;

I will be adding the rpcResponse.Error handling code, as demonstrated above, along with a config parameter to control whether an exception should be raised from the wrapper to the next release, unless there is something more you would like to add on top of that.

from bitcoinlib.

Hi,

Many thanks for the detailed answer.

Right now my project it on a halt, hence the late response…

I’m not a professional developer, therefore :

•      some of what you propose maybe seems awkward to me when maybe it shouldn’t.
  

•      some of my thoughts may be nonsense to you. 
  

Here goes anyway:

I feel the idea that you get a real JSON response, that contains information (though only in the error section), is ending up as an exception, is weird to me. It’s as if you are overriding the protocol behavior to some simplified other thing.

I would rather get the full object of Result+Error, which contain the collections, then the collection themselves, if there is a high risk of getting errors (and with my experience with wallets, there are risks of errors, and I would want to propagate as much of it as possible to the application).

Since others may think differently, and maybe this makes sense in some cases only, my recommendation would be to offer alternative methods that return a full response object (containing result with relevant collection inside + string error), and not the underlying collection that comes in case of success

I guess you’d know the elegant way of implementing it, or completely ignore if it does not make any sense to you.

As for AppDomain: AppDomain.CurrentDomain.FirstChanceException += FirstChanceExceptionHandler. I’m not sure I know how this would work, but if this behave like some kind of a sink, where I can log that the exception occurred, it won’t really help.

Before I decided the right way to use your code was without any changes to it, I was doing the following:

public T MakeRequest(RpcMethods rpcMethod, params object[] parameters)

    {

        JsonRpcResponse<T> rpcResponse = MakeRpcRequest<T>(new JsonRpcRequest(1, rpcMethod.ToString(), parameters), 0);

        if (rpcResponse == null)

            return default(T);

        else

            return rpcResponse.Result;

}

Please consider, maybe throwing the exception with the Error inside and then returning default(T), would be a good compromise

Regards,

Guy

From: George Kimionis [mailto:[email protected]]
Sent: Sunday, July 6, 2014 4:35 PM
To: GeorgeKimionis/BitcoinLib
Cc: 3strategy
Subject: Re: [BitcoinLib] Issue? / workaround inside public T MakeRequest !? (#14)

It all has to do with what you are trying to achieve. The way it is currently designed, your app will not get the rpcResponse.Error details, however an exception will be raised (try adding a FirstChanceException handler to your AppDomain: AppDomain.CurrentDomain.FirstChanceException += FirstChanceExceptionHandler;), so as long as you perform your crucial RPC calls in a Try/Catch block the current implementation will serve as a production-level framework for your app. If on the other hand you believe that the rpcResponse.Error's content is crucial for your app then you can throw an exception (or even execute any custom exception handling code) as-it-happens:

public T MakeRequest<T>(RpcMethods rpcMethod, params object[] parameters)
{
    JsonRpcResponse<T> rpcResponse = MakeRpcRequest<T>(new JsonRpcRequest(1, rpcMethod.ToString(), parameters), 0);

    if (!String.IsNullOrWhiteSpace(rpcResponse.Error))
    {
        throw new Exception(rpcResponse.Error);
    }

    return rpcResponse.Result;
}

For RPC methods that return String or Object, this will suffice:
return String.IsNullOrEmpty(rpcResponse.Error) ? rpcResponse.Result : (T)(object)rpcResponse.Error;

I will be adding the rpcResponse.Error handling code, as demonstrated above, along with a config parameter to control whether an exception should be raised from the wrapper to the next release, unless there is something more you would like to add on top of that.

—
Reply to this email directly or view it on GitHub #14 (comment) .

from bitcoinlib.

This is now resolved with the use of RpcInternalServerErrorException() that handles internal server error exceptions and rebroadcast both the error message as well as the RPC error code.

from bitcoinlib.