Skip to content

Commit

Permalink
Fix confusing errors in custom clients (#338)
Browse files Browse the repository at this point in the history
* [examples] Fix custom client to work with no query parameters

* [core] Ensure appropriate API type displayed on HTTP errors

Right now, indexer errors that are other HTTP errors,
say it's a Faucet error, which is confusing.  This
ensures that the types are known ahead of time, and
properly propagated.
  • Loading branch information
gregnazario authored Mar 26, 2024
1 parent 5376192 commit c2774d9
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 47 deletions.
17 changes: 15 additions & 2 deletions examples/typescript/custom_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ export async function fetchCustomClient<Req, Res>(requestOptions: ClientRequest<
method,
};

const response = await fetch(`${url}?${params}`, request);
let path = url;
if (params) {
path = `${url}?${params}`;
}

const response = await fetch(path, request);
const data = await response.json();
return {
status: response.status,
Expand Down Expand Up @@ -60,7 +65,12 @@ export async function superagentCustomClient<Req, Res>(
method,
};

const response = await superagent.get(`${url}?${params}`, request);
let path = url;
if (params) {
path = `${url}?${params}`;
}

const response = await superagent.get(path, request);
return {
status: response.status,
statusText: response.statusText,
Expand Down Expand Up @@ -92,6 +102,9 @@ const example = async () => {

const chainInfo = await aptos.getLedgerInfo();
console.log(`${JSON.stringify(chainInfo)}`);

const latestVersion = await aptos.getIndexerLastSuccessVersion();
console.log(`Latest indexer version: ${latestVersion}`);
}

// Call the inner functions
Expand Down
27 changes: 0 additions & 27 deletions src/api/aptosConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,31 +95,4 @@ export class AptosConfig {
throw Error(`apiType ${apiType} is not supported`);
}
}

/**
* Checks if the URL is a known indexer endpoint
*
* @internal
* */
isIndexerRequest(url: string): boolean {
return NetworkToIndexerAPI[this.network] === url;
}

/**
* Checks if the URL is a known fullnode endpoint
*
* @internal
* */
isFullnodeRequest(url: string): boolean {
return NetworkToNodeAPI[this.network] === url;
}

/**
* Checks if the URL is a known faucet endpoint
*
* @internal
* */
isFaucetRequest(url: string): boolean {
return NetworkToFaucetAPI[this.network] === url;
}
}
16 changes: 7 additions & 9 deletions src/client/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
import { AptosConfig } from "../api/aptosConfig";
import { AptosApiError, AptosResponse } from "./types";
import { VERSION } from "../version";
import { AptosRequest, MimeType, ClientRequest, ClientResponse, Client, AnyNumber } from "../types";
import { AnyNumber, AptosRequest, Client, ClientRequest, ClientResponse, MimeType } from "../types";
import { AptosApiType } from "../utils";

/**
* Meaningful errors map
Expand Down Expand Up @@ -63,6 +64,7 @@ export async function request<Req, Res>(options: ClientRequest<Req>, client: Cli
export async function aptosRequest<Req extends {}, Res extends {}>(
options: AptosRequest,
aptosConfig: AptosConfig,
apiType: AptosApiType,
): Promise<AptosResponse<Req, Res>> {
const { url, path } = options;
const fullUrl = path ? `${url}/${path}` : url;
Expand All @@ -85,7 +87,7 @@ export async function aptosRequest<Req extends {}, Res extends {}>(

// to support both fullnode and indexer responses,
// check if it is an indexer query, and adjust response.data
if (aptosConfig.isIndexerRequest(url)) {
if (apiType === AptosApiType.INDEXER) {
const indexerResponse = result.data as any;
// Handle Indexer general errors
if (indexerResponse.errors) {
Expand Down Expand Up @@ -115,11 +117,7 @@ export async function aptosRequest<Req extends {}, Res extends {}>(
errorMessage = `Unhandled Error ${result.status} : ${result.statusText}`;
}

// Since we already checked if it is an Indexer request, here we can be sure
// it either Fullnode or Faucet request
throw new AptosApiError(
options,
result,
`${aptosConfig.isFullnodeRequest(url) ? "Fullnode" : "Faucet"} error: ${errorMessage}`,
);
// We have to explicitly check for all request types, because if the error is a non-indexer error, but
// comes from an indexer request (e.g. 404), we'll need to mention it appropriately
throw new AptosApiError(options, result, `${apiType} error: ${errorMessage}`);
}
1 change: 1 addition & 0 deletions src/client/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export async function get<Req extends {}, Res extends {}>(
},
},
aptosConfig,
options.type,
);
}

Expand Down
1 change: 1 addition & 0 deletions src/client/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export async function post<Req extends {}, Res extends {}>(
overrides,
},
aptosConfig,
options.type,
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/utils/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
* Type of API endpoint for request routing
*/
export enum AptosApiType {
FULLNODE,
INDEXER,
FAUCET,
FULLNODE = "Fullnode",
INDEXER = "Indexer",
FAUCET = "Faucet",
}

/**
Expand Down
21 changes: 15 additions & 6 deletions tests/e2e/client/aptosRequest.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import {
aptosRequest,
Account,
Aptos,
AptosApiError,
AptosApiType,
AptosConfig,
aptosRequest,
generateSignedTransaction,
GraphqlQuery,
Network,
NetworkToIndexerAPI,
NetworkToNodeAPI,
Aptos,
Account,
U64,
GraphqlQuery,
NetworkToIndexerAPI,
generateSignedTransaction,
} from "../../../src";
import { VERSION } from "../../../src/version";
import { longTestTimeout } from "../../unit/helper";
Expand Down Expand Up @@ -50,6 +51,7 @@ describe("aptos request", () => {
overrides: { HEADERS: { my: "header" } },
},
config,
AptosApiType.FULLNODE,
);
expect(response.config.headers).toHaveProperty("x-aptos-client", `aptos-typescript-sdk/${VERSION}`);
expect(response.config.headers).toHaveProperty("my", "header");
Expand Down Expand Up @@ -77,6 +79,7 @@ describe("aptos request", () => {
originMethod: "test when token is set",
},
config,
AptosApiType.FULLNODE,
);
expect(response.config.headers).toHaveProperty("authorization", "Bearer my-api-key");
} catch (error: any) {
Expand All @@ -102,6 +105,7 @@ describe("aptos request", () => {
originMethod: "test fullnode 200 status",
},
config,
AptosApiType.FULLNODE,
);
expect(response).toHaveProperty("data", {
sequence_number: "0",
Expand Down Expand Up @@ -129,6 +133,7 @@ describe("aptos request", () => {
originMethod: "test 400 status",
},
config,
AptosApiType.FULLNODE,
);
} catch (error: any) {
expect(error).toBeInstanceOf(AptosApiError);
Expand Down Expand Up @@ -164,6 +169,7 @@ describe("aptos request", () => {
originMethod: "test 404 status",
},
config,
AptosApiType.FULLNODE,
);
} catch (error: any) {
expect(error).toBeInstanceOf(AptosApiError);
Expand Down Expand Up @@ -205,6 +211,7 @@ describe("aptos request", () => {
contentType: "application/x.aptos.signed_transaction+bcs",
},
config,
AptosApiType.FULLNODE,
);
} catch (error: any) {
expect(error).toBeInstanceOf(AptosApiError);
Expand Down Expand Up @@ -252,6 +259,7 @@ describe("aptos request", () => {
originMethod: "test indexer 200 status",
},
config,
AptosApiType.INDEXER,
);
expect(response).toHaveProperty("data", {
ledger_infos: [
Expand Down Expand Up @@ -288,6 +296,7 @@ describe("aptos request", () => {
originMethod: "test indexer 400 status",
},
config,
AptosApiType.INDEXER,
);
} catch (error: any) {
expect(error).toBeInstanceOf(AptosApiError);
Expand Down

0 comments on commit c2774d9

Please sign in to comment.