Skip to content

Commit

Permalink
Wrapper around native fetch to improve error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig committed Jul 28, 2023
1 parent 8f4df87 commit cb4e403
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 36 deletions.
9 changes: 2 additions & 7 deletions packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {ErrorAborted, Logger, TimeoutError, enhanceFetchErrors} from "@lodestar/utils";
import {ErrorAborted, Logger, TimeoutError, fetch, isFetchAbortError} from "@lodestar/utils";
import {ReqGeneric, RouteDef} from "../index.js";
import {ApiClientResponse, ApiClientSuccessResponse} from "../../interfaces.js";
import {stringifyQuery, urlJoin} from "./format.js";
Expand Down Expand Up @@ -301,7 +301,7 @@ export class HttpClient implements IHttpClient {
} catch (e) {
this.metrics?.requestErrors.inc({routeId});

if (isAbortedError(e as Error)) {
if (isFetchAbortError(e)) {
if (signalGlobal?.aborted) {
throw new ErrorAborted("REST client");
} else if (controller.signal.aborted) {
Expand All @@ -310,7 +310,6 @@ export class HttpClient implements IHttpClient {
throw Error("Unknown aborted error");
}
} else {
enhanceFetchErrors(e);
throw e;
}
} finally {
Expand All @@ -322,10 +321,6 @@ export class HttpClient implements IHttpClient {
}
}

function isAbortedError(e: Error): boolean {
return e.name === "AbortError";
}

function getErrorMessage(errBody: string): string {
try {
const errJson = JSON.parse(errBody) as {message: string};
Expand Down
1 change: 1 addition & 0 deletions packages/api/test/utils/fetchOpenApiSpec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from "node:fs";
import path from "node:path";
import {fetch} from "@lodestar/utils";
import {OpenApiFile, OpenApiJson} from "./parseOpenApiSpec.js";

/* eslint-disable no-console */
Expand Down
8 changes: 1 addition & 7 deletions packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {ErrorAborted, TimeoutError, enhanceFetchErrors, retry} from "@lodestar/utils";
import {ErrorAborted, TimeoutError, retry, fetch} from "@lodestar/utils";
import {IGauge, IHistogram} from "../../metrics/interface.js";
import {IJson, RpcPayload} from "../interface.js";
import {encodeJwtToken} from "./jwt.js";
Expand Down Expand Up @@ -187,13 +187,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 fetch throws with `TypeError: Failed to parse URL from undefined`
// Throw a better error instead
if (!url) throw Error(`Empty or undefined JSON RPC HTTP client url: ${url}`);

// fetch() throws for network errors:
// - cause: Error: getaddrinfo ENOTFOUND missing-url.com

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

Expand Down Expand Up @@ -250,7 +245,6 @@ export class JsonRpcHttpClient implements IJsonRpcHttpClient {
throw new TimeoutError("request");
}
} else {
enhanceFetchErrors(e);
throw e;
}
} finally {
Expand Down
4 changes: 2 additions & 2 deletions packages/beacon-node/src/execution/engine/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ export function getExecutionEngineState({
return ExecutionEngineState.OFFLINE;
}

if (payloadError && isFetchError(payloadError) && fatalErrorCodes.includes(payloadError.cause.code)) {
if (payloadError && isFetchError(payloadError) && fatalErrorCodes.includes(payloadError.code)) {
return ExecutionEngineState.OFFLINE;
}

if (payloadError && isFetchError(payloadError) && connectionErrorCodes.includes(payloadError.cause.code)) {
if (payloadError && isFetchError(payloadError) && connectionErrorCodes.includes(payloadError.code)) {
return ExecutionEngineState.AUTH_FAILED;
}

Expand Down
3 changes: 1 addition & 2 deletions packages/beacon-node/src/monitoring/service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Registry} from "prom-client";
import {ErrorAborted, Logger, TimeoutError, enhanceFetchErrors} from "@lodestar/utils";
import {ErrorAborted, Logger, TimeoutError, fetch} from "@lodestar/utils";
import {RegistryMetricCreator} from "../metrics/index.js";
import {HistogramExtra} from "../metrics/utils/histogram.js";
import {defaultMonitoringOptions, MonitoringOptions} from "./options.js";
Expand Down Expand Up @@ -180,7 +180,6 @@ export class MonitoringService {

if (!signal.aborted) {
// error was thrown by fetch
enhanceFetchErrors(e);
throw e;
}

Expand Down
1 change: 1 addition & 0 deletions packages/beacon-node/test/e2e/api/impl/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {fetch} from "@lodestar/utils";
import {ForkName, activePreset} from "@lodestar/params";
import {chainConfig} from "@lodestar/config/default";
import {ethereumConsensusSpecsTests} from "../../../spec/specTestVersioning.js";
Expand Down
23 changes: 17 additions & 6 deletions packages/beacon-node/test/e2e/eth1/jsonRpcHttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import "mocha";
import crypto from "node:crypto";
import http from "node:http";
import {expect} from "chai";
import {FetchError} from "@lodestar/utils";
import {JsonRpcHttpClient} from "../../../src/eth1/provider/jsonRpcHttpClient.js";
import {getGoerliRpcUrl} from "../../testParams.js";
import {RpcPayload} from "../../../src/eth1/interface.js";
Expand All @@ -22,6 +23,7 @@ describe("eth1 / jsonRpcHttpClient", function () {
abort?: true;
timeout?: number;
error: any;
errorCode?: string;
}[] = [
// // NOTE: This DNS query is very expensive, all cache miss. So it can timeout the tests and cause false positives
// {
Expand All @@ -33,13 +35,15 @@ describe("eth1 / jsonRpcHttpClient", function () {
id: "Bad subdomain",
// Use random bytes to ensure no collisions
url: `https://${randomHex}.infura.io`,
error: "getaddrinfo ENOTFOUND",
error: "",
errorCode: "ENOTFOUND",
},
{
id: "Bad port",
url: `http://localhost:${port + 1}`,
requestListener: (req, res) => res.end(),
error: "connect ECONNREFUSED",
error: "",
errorCode: "ECONNREFUSED",
},
{
id: "Not a JSON RPC endpoint",
Expand Down Expand Up @@ -122,7 +126,6 @@ describe("eth1 / jsonRpcHttpClient", function () {

for (const testCase of testCases) {
const {id, requestListener, abort, timeout} = testCase;
const error = testCase.error as Error;
let {url, payload} = testCase;

it(id, async function () {
Expand All @@ -148,7 +151,13 @@ describe("eth1 / jsonRpcHttpClient", function () {
const controller = new AbortController();
if (abort) setTimeout(() => controller.abort(), 50);
const eth1JsonRpcClient = new JsonRpcHttpClient([url], {signal: controller.signal});
await expect(eth1JsonRpcClient.fetch(payload, {timeout})).to.be.rejectedWith(error);
await expect(eth1JsonRpcClient.fetch(payload, {timeout})).to.be.rejected.then((error) => {
if (testCase.errorCode) {
expect((error as FetchError).code).to.be.equal(testCase.errorCode);
} else {
expect((error as Error).message).to.include(testCase.error);
}
});
});
}
});
Expand Down Expand Up @@ -210,8 +219,10 @@ describe("eth1 / jsonRpcHttpClient - with retries", function () {
return true;
},
})
).to.be.rejectedWith("connect ECONNREFUSED");
expect(retryCount).to.be.equal(retryAttempts, "connect ECONNREFUSED should be retried before failing");
).to.be.rejected.then((error) => {
expect((error as FetchError).code).to.be.equal("ECONNREFUSED");
});
expect(retryCount).to.be.equal(retryAttempts, "code ECONNREFUSED should be retried before failing");
});

it("should retry 404", async function () {
Expand Down
1 change: 1 addition & 0 deletions packages/beacon-node/test/unit/metrics/server/http.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {fetch} from "@lodestar/utils";
import {getHttpMetricsServer, HttpMetricsServer} from "../../../../src/metrics/index.js";
import {testLogger} from "../../../utils/logger.js";
import {createMetricsTest} from "../utils.js";
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/test/unit/monitoring/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe("monitoring / service", () => {

await service.send();

assertError({message: `connect ECONNREFUSED ${new URL(endpoint).host}`});
assertError({message: `Request to ${endpoint} failed, reason: connect ECONNREFUSED ${new URL(endpoint).host}`});
});

it("should abort pending requests if timeout is reached", async () => {
Expand Down
128 changes: 117 additions & 11 deletions packages/utils/src/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,128 @@
export type FetchError = Error & {
/**
* Wrapper around native fetch to improve error handling
*/
async function wrappedFetch(url: string | URL, init?: RequestInit): Promise<Response> {
try {
return await fetch(url, init);
} catch (e) {
throw new FetchError(e, url);
}
}

export {wrappedFetch as fetch};

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

export function isFetchAbortError(e: unknown): e is FetchError {
return e instanceof FetchError && e.type === "aborted";
}

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

export type FetchErrorCause = NativeFetchSystemError["cause"] | NativeFetchInputError["cause"];

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

constructor(e: unknown, url: string | URL) {
if (isNativeSystemFetchError(e)) {
super(`Request to ${url.toString()} failed, reason: ${e.cause.message}`);
this.type = "system";
this.code = e.cause.code;
this.cause = e.cause;
} else if (isNativeFetchInputError(e)) {
super(e.message);
this.type = "input";
this.code = e.cause.code;
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;
}
}

/**
* ```
* 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'
* ```
*/
type NativeFetchSystemError = Error & {
cause: Error & {
errno: string;
code: string;
syscall: string;
address?: string;
port?: string;
hostname?: string;
};
};

export function isFetchError(error: unknown): error is FetchError {
/**
* ```
* TypeError: Failed to parse URL from invalid-url
* [cause]: TypeError [ERR_INVALID_URL]: Invalid URL
* input: 'invalid-url',
* code: 'ERR_INVALID_URL'
* ```
*/
type NativeFetchInputError = Error & {
cause: Error & {
input: unknown;
code: string;
};
};

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

function isNativeSystemFetchError(e: unknown): e is NativeFetchSystemError {
return (
error instanceof Error &&
(error as FetchError).cause instanceof Error &&
(error as FetchError).cause.errno !== undefined &&
(error as FetchError).cause.code !== undefined
e instanceof Error &&
(e as NativeFetchSystemError).cause instanceof Error &&
(e as NativeFetchSystemError).cause.code !== undefined &&
(e as NativeFetchSystemError).cause.syscall !== undefined
);
}

export function enhanceFetchErrors(error: unknown): void {
if (isFetchError(error)) {
// Override message with cause.message to get more detailed errors
error.message = error.cause.message;
}
function isNativeFetchInputError(e: unknown): e is NativeFetchInputError {
return (
e instanceof Error &&
(e as NativeFetchInputError).cause instanceof Error &&
(e as NativeFetchInputError).cause.code !== undefined &&
(e as NativeFetchInputError).cause.input !== undefined
);
}

function isNativeFetchAbortError(e: unknown): e is NativeFetchAbortError {
return e instanceof DOMException && e.name === "AbortError";
}
1 change: 1 addition & 0 deletions packages/validator/src/util/externalSignerClient.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ContainerType, toHexString, ValueOf} from "@chainsafe/ssz";
import {phase0, altair, capella} from "@lodestar/types";
import {ForkSeq} from "@lodestar/params";
import {fetch} from "@lodestar/utils";
import {ValidatorRegistrationV1} from "@lodestar/types/bellatrix";
import {BeaconConfig} from "@lodestar/config";
import {computeEpochAtSlot, blindedOrFullBlockToHeader} from "@lodestar/state-transition";
Expand Down

0 comments on commit cb4e403

Please sign in to comment.