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

refactor: switch to native fetch implementation #5811

Merged
merged 13 commits into from
Jul 31, 2023
1 change: 0 additions & 1 deletion packages/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
"@lodestar/params": "^1.9.2",
"@lodestar/types": "^1.9.2",
"@lodestar/utils": "^1.9.2",
"cross-fetch": "^3.1.8",
"eventsource": "^2.0.2",
"qs": "^6.11.1"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export {
HttpError,
ApiError,
Metrics,
FetchError,
isFetchError,
fetch,
} from "./utils/client/index.js";
export * from "./utils/routes.js";

Expand Down
137 changes: 137 additions & 0 deletions packages/api/src/utils/client/fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* Native fetch with transparent and consistent error handling
*
* [MDN Reference](https://developer.mozilla.org/docs/Web/API/fetch)
*/
async function wrappedFetch(url: string | URL, init?: RequestInit): Promise<Response> {
try {
return await fetch(url, init);
} catch (e) {
throw new FetchError(url, e);
}
}

export {wrappedFetch as fetch};

export function isFetchError(e: unknown): e is FetchError {
return e instanceof FetchError;
}

export type FetchErrorType = "failed" | "input" | "aborted" | "unknown";

type FetchErrorCause = NativeFetchFailedError["cause"] | NativeFetchInputError["cause"];

export class FetchError extends Error {
type: FetchErrorType;
code: string;
cause?: FetchErrorCause;

constructor(url: string | URL, e: unknown) {
if (isNativeFetchFailedError(e)) {
super(`Request to ${url.toString()} failed, reason: ${e.cause.message}`);
this.type = "failed";
this.code = e.cause.code || "ERR_FETCH_FAILED";
this.cause = e.cause;
} else if (isNativeFetchInputError(e)) {
// For input errors the e.message is more detailed
super(e.message);
this.type = "input";
this.code = e.cause.code || "ERR_INVALID_INPUT";
this.cause = e.cause;
} else if (isNativeFetchAbortError(e)) {
super(`Request to ${url.toString()} was aborted`);
this.type = "aborted";
this.code = "ERR_ABORTED";
} else {
super((e as Error).message);
this.type = "unknown";
this.code = "ERR_UNKNOWN";
}
this.name = this.constructor.name;
}
}

type NativeFetchError = TypeError & {
cause: Error & {
code?: string;
};
};

/**
* ```
* TypeError: fetch failed
* cause: Error: connect ECONNREFUSED 127.0.0.1:9596
* errno: -111,
* code: 'ECONNREFUSED',
* syscall: 'connect',
* address: '127.0.0.1',
* port: 9596
* ---------------------------
* TypeError: fetch failed
* cause: Error: getaddrinfo ENOTFOUND non-existent-domain
* errno: -3008,
* code: 'ENOTFOUND',
* syscall: 'getaddrinfo',
* hostname: 'non-existent-domain'
* ---------------------------
* TypeError: fetch failed
* cause: SocketError: other side closed
* code: 'UND_ERR_SOCKET',
* socket: {}
* ---------------------------
* TypeError: fetch failed
* cause: Error: unknown scheme
* [cause]: undefined
* ```
*/
type NativeFetchFailedError = NativeFetchError & {
message: "fetch failed";
cause: {
errno?: string;
syscall?: string;
address?: string;
port?: string;
hostname?: string;
socket?: object;
[prop: string]: unknown;
};
};

/**
* ```
* TypeError: Failed to parse URL from invalid-url
* [cause]: TypeError [ERR_INVALID_URL]: Invalid URL
* input: 'invalid-url',
* code: 'ERR_INVALID_URL'
* ```
*/
type NativeFetchInputError = NativeFetchError & {
cause: {
input: unknown;
};
};

/**
* ```
* DOMException [AbortError]: This operation was aborted
* ```
*/
type NativeFetchAbortError = DOMException & {
name: "AbortError";
};

function isNativeFetchError(e: unknown): e is NativeFetchError {
return e instanceof TypeError && (e as NativeFetchError).cause instanceof Error;
}

function isNativeFetchFailedError(e: unknown): e is NativeFetchFailedError {
return isNativeFetchError(e) && (e as NativeFetchFailedError).message === "fetch failed";
}

function isNativeFetchInputError(e: unknown): e is NativeFetchInputError {
return isNativeFetchError(e) && (e as NativeFetchInputError).cause.input !== undefined;
}

function isNativeFetchAbortError(e: unknown): e is NativeFetchAbortError {
return e instanceof DOMException && e.name === "AbortError";
}
6 changes: 3 additions & 3 deletions packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {fetch} from "cross-fetch";
import {ErrorAborted, Logger, TimeoutError} from "@lodestar/utils";
import {ReqGeneric, RouteDef} from "../index.js";
import {ApiClientResponse, ApiClientSuccessResponse} from "../../interfaces.js";
import {fetch, isFetchError} from "./fetch.js";
import {stringifyQuery, urlJoin} from "./format.js";
import {Metrics} from "./metrics.js";
import {HttpStatusCode} from "./httpStatusCode.js";
Expand Down Expand Up @@ -261,7 +261,7 @@ export class HttpClient implements IHttpClient {
const timeout = setTimeout(() => controller.abort(), opts.timeoutMs ?? timeoutMs ?? this.globalTimeoutMs);

// Attach global signal to this request's controller
const onGlobalSignalAbort = controller.abort.bind(controller);
const onGlobalSignalAbort = (): void => controller.abort();
const signalGlobal = this.getAbortSignal?.();
signalGlobal?.addEventListener("abort", onGlobalSignalAbort);

Expand Down Expand Up @@ -323,7 +323,7 @@ export class HttpClient implements IHttpClient {
}

function isAbortedError(e: Error): boolean {
return e.name === "AbortError" || e.message === "The user aborted a request";
return isFetchError(e) && e.type === "aborted";
}

function getErrorMessage(errBody: string): string {
Expand Down
1 change: 1 addition & 0 deletions packages/api/src/utils/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./client.js";
export * from "./httpClient.js";
export * from "./fetch.js";
121 changes: 121 additions & 0 deletions packages/api/test/unit/client/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import crypto from "node:crypto";
import http from "node:http";
import {expect} from "chai";
import {FetchError, FetchErrorType, fetch} from "../../../src/utils/client/fetch.js";

describe("FetchError", function () {
const port = 37421;
const randomHex = crypto.randomBytes(32).toString("hex");

const testCases: {
id: string;
url?: string;
requestListener?: http.RequestListener;
abort?: true;
timeout?: number;
errorType: FetchErrorType;
errorCode: string;
expectCause: boolean;
}[] = [
{
id: "Bad domain",
// Use random bytes to ensure no collisions
url: `https://${randomHex}.infura.io`,
errorType: "failed",
errorCode: "ENOTFOUND",
expectCause: true,
},
{
id: "Bad port",
url: `http://localhost:${port + 1}`,
requestListener: (_req, res) => res.end(),
errorType: "failed",
errorCode: "ECONNREFUSED",
expectCause: true,
},
{
id: "Socket error",
requestListener: (_req, res) => res.socket?.destroy(),
errorType: "failed",
errorCode: "UND_ERR_SOCKET",
expectCause: true,
},
{
id: "Headers overflow",
requestListener: (_req, res) => {
res.setHeader("Large-Header", "a".repeat(1e6));
res.end();
},
errorType: "failed",
errorCode: "UND_ERR_HEADERS_OVERFLOW",
expectCause: true,
},
{
id: "Unknown scheme",
url: `httsp://localhost:${port}`,
errorType: "failed",
errorCode: "ERR_FETCH_FAILED",
expectCause: true,
},
{
id: "Invalid URL",
url: "invalid-url",
errorType: "input",
errorCode: "ERR_INVALID_URL",
expectCause: true,
},
{
id: "Aborted request",
abort: true,
requestListener: () => {
// leave the request open until aborted
},
errorType: "aborted",
errorCode: "ERR_ABORTED",
expectCause: false,
},
];

const afterHooks: (() => Promise<void>)[] = [];

afterEach(async function () {
while (afterHooks.length) {
const afterHook = afterHooks.pop();
if (afterHook)
await afterHook().catch((e: Error) => {
// eslint-disable-next-line no-console
console.error("Error in afterEach hook", e);
});
}
});

for (const testCase of testCases) {
const {id, url = `http://localhost:${port}`, requestListener, abort} = testCase;

it(id, async function () {
if (requestListener) {
const server = http.createServer(requestListener);
await new Promise<void>((resolve) => server.listen(port, resolve));
afterHooks.push(
() =>
new Promise((resolve, reject) =>
server.close((err) => {
if (err) reject(err);
else resolve();
})
)
);
}

const controller = new AbortController();
if (abort) setTimeout(() => controller.abort(), 20);
await expect(fetch(url, {signal: controller.signal})).to.be.rejected.then((error: FetchError) => {
expect(error.type).to.be.equal(testCase.errorType);
expect(error.code).to.be.equal(testCase.errorCode);
if (testCase.expectCause) {
expect(error.cause).to.be.instanceof(Error);
}
});
});
}
});
2 changes: 1 addition & 1 deletion packages/api/test/utils/fetchOpenApiSpec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from "node:fs";
import path from "node:path";
import fetch from "cross-fetch";
import {fetch} from "@lodestar/api";
import {OpenApiFile, OpenApiJson} from "./parseOpenApiSpec.js";

/* eslint-disable no-console */
Expand Down
1 change: 0 additions & 1 deletion packages/beacon-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
"@types/datastore-level": "^3.0.0",
"buffer-xor": "^2.0.2",
"c-kzg": "^2.1.0",
"cross-fetch": "^3.1.8",
"datastore-core": "^9.1.1",
"datastore-level": "^10.1.1",
"deepmerge": "^4.3.1",
Expand Down
20 changes: 1 addition & 19 deletions packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
// Uses cross-fetch for browser + NodeJS cross compatibility
// Note: isomorphic-fetch is not well mantained and does not support abort signals
import fetch from "cross-fetch";

import {fetch} from "@lodestar/api";
import {ErrorAborted, TimeoutError, retry} from "@lodestar/utils";
import {IGauge, IHistogram} from "../../metrics/interface.js";
import {IJson, RpcPayload} from "../interface.js";
Expand All @@ -12,16 +9,6 @@ import {encodeJwtToken} from "./jwt.js";
const maxStringLengthToPrint = 500;
const REQUEST_TIMEOUT = 30 * 1000;

// As we are using `cross-fetch` which does not support for types for errors
// We can't use `node-fetch` for browser compatibility
export type FetchError = {
errno: string;
code: string;
};

export const isFetchError = (error: unknown): error is FetchError =>
(error as FetchError) !== undefined && "code" in (error as FetchError) && "errno" in (error as FetchError);

interface RpcResponse<R> extends RpcResponseError {
result?: R;
}
Expand Down Expand Up @@ -201,13 +188,8 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
* Fetches JSON and throws detailed errors in case the HTTP request is not ok
*/
private async fetchJsonOneUrl<R, T = unknown>(url: string, json: T, opts?: ReqOpts): Promise<R> {
// If url is undefined node-fetch throws with `TypeError: Only absolute URLs are supported`
// Throw a better error instead
if (!url) throw Error(`Empty or undefined JSON RPC HTTP client url: ${url}`);

// fetch() throws for network errors:
// - request to http://missing-url.com/ failed, reason: getaddrinfo ENOTFOUND missing-url.com

const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), opts?.timeout ?? this.opts?.timeout ?? REQUEST_TIMEOUT);

Expand Down
3 changes: 2 additions & 1 deletion packages/beacon-node/src/execution/engine/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {isFetchError} from "@lodestar/api";
import {IJson, RpcPayload} from "../../eth1/interface.js";
import {IJsonRpcHttpClient, isFetchError} from "../../eth1/provider/jsonRpcHttpClient.js";
import {IJsonRpcHttpClient} from "../../eth1/provider/jsonRpcHttpClient.js";
import {ExecutePayloadStatus, ExecutionEngineState} from "./interface.js";

export type JsonRpcBackend = {
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/src/monitoring/service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fetch from "cross-fetch";
import {Registry} from "prom-client";
import {fetch} from "@lodestar/api";
import {ErrorAborted, Logger, TimeoutError} from "@lodestar/utils";
import {RegistryMetricCreator} from "../metrics/index.js";
import {HistogramExtra} from "../metrics/utils/histogram.js";
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/test/e2e/api/impl/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fetch from "cross-fetch";
import {fetch} from "@lodestar/api";
import {ForkName, activePreset} from "@lodestar/params";
import {chainConfig} from "@lodestar/config/default";
import {ethereumConsensusSpecsTests} from "../../../spec/specTestVersioning.js";
Expand Down
Loading
Loading