[v2] Use RpcResponse<T> instead of exception-driven development

[skynetcap]

Discuss:

For Version 2, I'd like to remove the required try-catch'ing of RpcException. I've received feedback that it's a bad pattern to catch exceptions as a requirement. I believe changing the method signatures on RpcApi would break all clients, thus a major version (v2) would have these changes. I'm strongly against breaking clients, but v2 is needed. I could be wrong on any of these so please opine.

Example:

Current: (requires a try-catch block which is deal-breaking for some)

try {
    long balance = client.getApi().getBalance(...);
} catch (RpcException ex) {
    log.error(ex);
}

Future: (does not require exceptions)

RpcResponse<Long> response = client.getApi().getBalance();
if (response.isSuccessful()) {
    // happy path
    long balance = response.getValue();
} else {
    // handle/log error gracefully
    log.error("...");
}

[skynetcap]

Commentary from the AI...

Analysis of Proposed Changes for solanaj v2

Overview

This document analyzes the proposed changes for version 2 of the solanaj library as discussed in GitHub issue #50.

Key Points

1. Moving away from exception-driven development:
   The proposal to replace the current exception-based approach with an RpcResponse<T> return type is a sound design decision. Exception-driven flow control can indeed be considered a bad practice, especially when exceptions are used for expected scenarios rather than truly exceptional cases.

2. Improved error handling:
   The proposed RpcResponse<T> approach allows for more granular and explicit error handling. This gives developers using the library more control over how they want to handle different error scenarios, rather than forcing them to catch a generic RpcException.

3. Breaking changes and versioning:
   It's commendable that this is being considered as a major version change (v2). This follows semantic versioning principles, where breaking changes should result in a major version bump. This approach respects existing users of the library while allowing for significant improvements.

4. Code readability and maintainability:
   The proposed change would likely lead to more readable and maintainable code for users of the library. The example provided in the issue demonstrates this clearly:

   RpcResponse<Long> response = client.getApi().getBalance();
   if (response.getError() != null) {
       // handle gracefully or bail out
   }
   long balance = response.getValue();

   This is more explicit and easier to understand than wrapping every call in a try-catch block.

5. Flexibility for library users:
   This change gives users of the library more flexibility in how they want to handle errors. They can choose to handle errors immediately, propagate them, or even revert to throwing exceptions if that's what they prefer.

Commentary

I believe these changes should be implemented. They align with modern Java programming practices and will likely make the library more user-friendly and robust. However, it's important to:

1. Clearly document the changes and provide migration guides for users moving from v1 to v2.
2. Consider providing utility methods to ease the transition, such as methods that throw exceptions for users who prefer that style.
3. Ensure that the RpcResponse<T> class is well-designed, possibly including methods like isSuccess(), getError(), and getValue() for convenience.

Conclusion

Overall, this change seems to be a positive step forward for the solanaj library, improving its design and usability while respecting existing users through proper versioning.

[yanyan1716330643]

System.out.println(client.getApi().getVersion());//2.0.8
//getLatestBlockhash ， the RPC method returns a structure that is no longer the same as before, as follows:

{
    "jsonrpc": "2.0",
    "result": {
        "context": {
            "apiVersion": "2.0.8",
            "slot": 325632991
        },
        "value": {
            "blockhash": "GHuhezBLcVjRj6VKsTxqcNpfBqpYUBFFFGgzNcFyLmzb",
            "lastValidBlockHeight": 313850149
        }
    },
    "id": "ee47c1d7-b320-4925-929d-99987259a841"
}

the value structure is err,Many methods have this version issue

       "value": {
            "blockhash": "GHuhezBLcVjRj6VKsTxqcNpfBqpYUBFFFGgzNcFyLmzb",
            "lastValidBlockHeight": 313850149
        }

[yanyan1716330643]

@Getter
@ToString
public class RecentBlockhash extends RpcResultObject {

    @Getter
    @ToString
    public static class FeeCalculator {

        @Json(name = "lamportsPerSignature")
        private double lamportsPerSignature;
    }

    @Getter
    @ToString
    public static class Value {
        @Json(name = "blockhash")
        private String blockhash;

        @Json(name = "feeCalculator")
        private FeeCalculator feeCalculator;
    }

    @Json(name = "value")
    private Value value;
}

this is the java bean

[skynetcap]

@yanyan1716330643 taking a look and fixing. thank you for the report

[skynetcap]

Fixed in #51.

Releasing with 1.18.2 shortly.

[yanyan1716330643]

think you,I will use more of your code and provide feedback to you. Thank you for the open source Solana SDK for Java

[skynetcap]

1.18.2 has been released. Let me know if that works. If there's an issue, open a separate ticket. Thanks!