From c2774d9030f6b3011fffc1ce5c60c8a927d9032a Mon Sep 17 00:00:00 2001 From: Greg Nazario Date: Tue, 26 Mar 2024 10:38:34 -0700 Subject: [PATCH] Fix confusing errors in custom clients (#338) * [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. --- examples/typescript/custom_client.ts | 17 +++++++++++++++-- src/api/aptosConfig.ts | 27 --------------------------- src/client/core.ts | 16 +++++++--------- src/client/get.ts | 1 + src/client/post.ts | 1 + src/utils/const.ts | 6 +++--- tests/e2e/client/aptosRequest.test.ts | 21 +++++++++++++++------ 7 files changed, 42 insertions(+), 47 deletions(-) diff --git a/examples/typescript/custom_client.ts b/examples/typescript/custom_client.ts index 8f30bb992..197df745c 100644 --- a/examples/typescript/custom_client.ts +++ b/examples/typescript/custom_client.ts @@ -32,7 +32,12 @@ export async function fetchCustomClient(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, @@ -60,7 +65,12 @@ export async function superagentCustomClient( 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, @@ -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 diff --git a/src/api/aptosConfig.ts b/src/api/aptosConfig.ts index 4e7e18506..4b0722ec1 100644 --- a/src/api/aptosConfig.ts +++ b/src/api/aptosConfig.ts @@ -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; - } } diff --git a/src/client/core.ts b/src/client/core.ts index 76eb944da..12534e5f2 100644 --- a/src/client/core.ts +++ b/src/client/core.ts @@ -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 @@ -63,6 +64,7 @@ export async function request(options: ClientRequest, client: Cli export async function aptosRequest( options: AptosRequest, aptosConfig: AptosConfig, + apiType: AptosApiType, ): Promise> { const { url, path } = options; const fullUrl = path ? `${url}/${path}` : url; @@ -85,7 +87,7 @@ export async function aptosRequest( // 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) { @@ -115,11 +117,7 @@ export async function aptosRequest( 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}`); } diff --git a/src/client/get.ts b/src/client/get.ts index db244b155..c8f6cd5f0 100644 --- a/src/client/get.ts +++ b/src/client/get.ts @@ -68,6 +68,7 @@ export async function get( }, }, aptosConfig, + options.type, ); } diff --git a/src/client/post.ts b/src/client/post.ts index a04434e86..1caa5e11f 100644 --- a/src/client/post.ts +++ b/src/client/post.ts @@ -73,6 +73,7 @@ export async function post( overrides, }, aptosConfig, + options.type, ); } diff --git a/src/utils/const.ts b/src/utils/const.ts index 0fc44b4e7..248a5fd1e 100644 --- a/src/utils/const.ts +++ b/src/utils/const.ts @@ -5,9 +5,9 @@ * Type of API endpoint for request routing */ export enum AptosApiType { - FULLNODE, - INDEXER, - FAUCET, + FULLNODE = "Fullnode", + INDEXER = "Indexer", + FAUCET = "Faucet", } /** diff --git a/tests/e2e/client/aptosRequest.test.ts b/tests/e2e/client/aptosRequest.test.ts index 63f41e44d..4c7617407 100644 --- a/tests/e2e/client/aptosRequest.test.ts +++ b/tests/e2e/client/aptosRequest.test.ts @@ -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"; @@ -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"); @@ -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) { @@ -102,6 +105,7 @@ describe("aptos request", () => { originMethod: "test fullnode 200 status", }, config, + AptosApiType.FULLNODE, ); expect(response).toHaveProperty("data", { sequence_number: "0", @@ -129,6 +133,7 @@ describe("aptos request", () => { originMethod: "test 400 status", }, config, + AptosApiType.FULLNODE, ); } catch (error: any) { expect(error).toBeInstanceOf(AptosApiError); @@ -164,6 +169,7 @@ describe("aptos request", () => { originMethod: "test 404 status", }, config, + AptosApiType.FULLNODE, ); } catch (error: any) { expect(error).toBeInstanceOf(AptosApiError); @@ -205,6 +211,7 @@ describe("aptos request", () => { contentType: "application/x.aptos.signed_transaction+bcs", }, config, + AptosApiType.FULLNODE, ); } catch (error: any) { expect(error).toBeInstanceOf(AptosApiError); @@ -252,6 +259,7 @@ describe("aptos request", () => { originMethod: "test indexer 200 status", }, config, + AptosApiType.INDEXER, ); expect(response).toHaveProperty("data", { ledger_infos: [ @@ -288,6 +296,7 @@ describe("aptos request", () => { originMethod: "test indexer 400 status", }, config, + AptosApiType.INDEXER, ); } catch (error: any) { expect(error).toBeInstanceOf(AptosApiError);