Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Breaking change: decouple from "axios" #53

Closed
wants to merge 2 commits into from

Conversation

andreibancioiu
Copy link
Contributor

@andreibancioiu andreibancioiu commented Dec 20, 2023

While axios is a great library, we'd like to decouple sdk-network-providers from it - so that issues related to axios do not impact this package anymore.

Breaking change: the constructors ApiNetworkProvider and ProxyNetworkProvider have been altered (see snippet below).

Client code would now be responsible with providing a httpProvider when instantiating ApiNetworkProvider or ProxyNetworkProvider.

A httpProvider must satisfy the following interface:

export interface IHttpProvider {
    get(url: string): Promise<any>;
    post(url: string, payload: any): Promise<any>;
    // TBD: error handling
}

Such a httpProvider can be power, indeed, by axios. For example:

const apiProvider: INetworkProvider = new ApiNetworkProvider({
    baseUrl: "https://devnet-api.multiversx.com",
    httpProvider: createHttpProvider()
});

const proxyProvider: INetworkProvider = new ProxyNetworkProvider({
    baseUrl: "https://devnet-gateway.multiversx.com",
    httpProvider: createHttpProvider()
});

// Depends on:
// - https://www.npmjs.com/package/axios
// - https://www.npmjs.com/package/json-bigint
function createHttpProvider() {
    const axios = require("axios").default;
    const jsonBig = require("json-bigint")({ constructorAction: "ignore" });

    const axiosConfig = {
        timeout: 10000,
        // See: https://github.com/axios/axios/issues/983 regarding transformResponse
        // Very important, to properly handle responses bearing big integers.
        transformResponse: [
            function (data: any) {
                return jsonBig.parse(data);
            }
        ],
        headers: {
            "Content-Type": "application/json"
        }
    };

    return {
        async get(url: string) {
            return (await axios.get(url, axiosConfig)).data;
        },
        async post(url: string, payload: any) {
            return (await axios.post(url, payload, axiosConfig)).data;
        }
    };
}

This design change also allows the code that depends on ApiNetworkProvider or ProxyNetworkProvider to be tested in a less complex fashion - since passing a mock IHttpProvider would be much simpler than using special axios mock adapters.

@@ -133,3 +133,8 @@ export interface ITransaction {
}

export interface IAddress { bech32(): string; }

export interface IHttpProvider {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use generics here for those who know the response type:

export interface IHttpProvider {
    get<T = any>(url: string): Promise<T>;
    post<T = any>(url: string, payload: any): Promise<T>;
}

Copy link

@CiprianDraghici CiprianDraghici Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the consumer is responsible for error handling.
eg.

try {
// the actual request
} catch(e){ 
// do something the the error
}

@andreibancioiu
Copy link
Contributor Author

Will be closed (cancelled / postponed).

@andreibancioiu andreibancioiu deleted the decouple-from-axios branch October 21, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants