Skip to content

Commit

Permalink
Revert "refactor: switch to native fetch implementation (#5811)"
Browse files Browse the repository at this point in the history
This reverts commit 3e65be7.
  • Loading branch information
matthewkeil committed Aug 9, 2023
1 parent 664820a commit 9f34079
Show file tree
Hide file tree
Showing 19 changed files with 44 additions and 278 deletions.
1 change: 1 addition & 0 deletions packages/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"@lodestar/params": "^1.10.0",
"@lodestar/types": "^1.10.0",
"@lodestar/utils": "^1.10.0",
"cross-fetch": "^3.1.8",
"eventsource": "^2.0.2",
"qs": "^6.11.1"
},
Expand Down
3 changes: 0 additions & 3 deletions packages/api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ export {
HttpError,
ApiError,
Metrics,
FetchError,
isFetchError,
fetch,
} from "./utils/client/index.js";
export * from "./utils/routes.js";

Expand Down
137 changes: 0 additions & 137 deletions packages/api/src/utils/client/fetch.ts

This file was deleted.

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 = (): void => controller.abort();
const onGlobalSignalAbort = controller.abort.bind(controller);
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 isFetchError(e) && e.type === "aborted";
return e.name === "AbortError" || e.message === "The user aborted a request";
}

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

This file was deleted.

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 "@lodestar/api";
import fetch from "cross-fetch";
import {OpenApiFile, OpenApiJson} from "./parseOpenApiSpec.js";

/* eslint-disable no-console */
Expand Down
1 change: 1 addition & 0 deletions packages/beacon-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@
"@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: 19 additions & 1 deletion packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import {fetch} from "@lodestar/api";
// 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 {ErrorAborted, TimeoutError, retry} from "@lodestar/utils";
import {IGauge, IHistogram} from "../../metrics/interface.js";
import {IJson, RpcPayload} from "../interface.js";
Expand All @@ -9,6 +12,16 @@ 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 @@ -188,8 +201,13 @@ 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
8 changes: 6 additions & 2 deletions packages/beacon-node/src/execution/engine/utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {isFetchError} from "@lodestar/api";
import {isErrorAborted} from "@lodestar/utils";
import {IJson, RpcPayload} from "../../eth1/interface.js";
import {IJsonRpcHttpClient, ErrorJsonRpcResponse, HttpRpcError} from "../../eth1/provider/jsonRpcHttpClient.js";
import {
IJsonRpcHttpClient,
ErrorJsonRpcResponse,
HttpRpcError,
isFetchError,
} from "../../eth1/provider/jsonRpcHttpClient.js";
import {isQueueErrorAborted} from "../../util/queue/errors.js";
import {ExecutionPayloadStatus, ExecutionEngineState} from "./interface.js";

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 "@lodestar/api";
import fetch from "cross-fetch";
import {ForkName, activePreset} from "@lodestar/params";
import {chainConfig} from "@lodestar/config/default";
import {ethereumConsensusSpecsTests} from "../../../spec/specTestVersioning.js";
Expand Down
Loading

0 comments on commit 9f34079

Please sign in to comment.