diff --git a/core/src/analytics/analytics.ts b/core/src/analytics/analytics.ts index abcda816be..08d312ec82 100644 --- a/core/src/analytics/analytics.ts +++ b/core/src/analytics/analytics.ts @@ -23,7 +23,7 @@ import { Profile } from "../util/profiling" import { ModuleConfig } from "../config/module" import { UserResult } from "@garden-io/platform-api-types" import { uuidv4 } from "../util/random" -import { GardenError, NodeJSErrnoErrorCodes, StackTraceMetadata } from "../exceptions" +import { GardenError, NodeJSErrnoException, StackTraceMetadata } from "../exceptions" import { ActionConfigMap } from "../actions/types" import { actionKinds } from "../actions/types" import { getResultErrorProperties } from "./helpers" @@ -163,7 +163,7 @@ export type AnalyticsGardenErrorDetail = { /** * If this error was caused by an underlying NodeJSErrnoException, this will be the code. */ - code?: NodeJSErrnoErrorCodes + code?: NodeJSErrnoException["code"] stackTrace?: StackTraceMetadata } diff --git a/core/src/exceptions.ts b/core/src/exceptions.ts index 8caed98b79..246f91f354 100644 --- a/core/src/exceptions.ts +++ b/core/src/exceptions.ts @@ -14,19 +14,48 @@ import stripAnsi from "strip-ansi" import { Cycle } from "./graph/common" import indentString from "indent-string" import { constants } from "os" +import dns from "node:dns" + +// Unfortunately, NodeJS does not provide a list of all error codes, so we have to maintain this list manually. +// See https://nodejs.org/docs/latest-v18.x/api/dns.html#error-codes +const dnsErrorCodes = [ + dns.NODATA, + dns.FORMERR, + dns.SERVFAIL, + dns.NOTFOUND, + dns.NOTIMP, + dns.REFUSED, + dns.BADQUERY, + dns.BADNAME, + dns.BADFAMILY, + dns.BADRESP, + dns.CONNREFUSED, + dns.TIMEOUT, + dns.EOF, + dns.FILE, + dns.NOMEM, + dns.DESTRUCTION, + dns.BADSTR, + dns.BADFLAGS, + dns.NONAME, + dns.BADHINTS, + dns.NOTINITIALIZED, + dns.LOADIPHLPAPI, + dns.ADDRGETNETWORKPARAMS, + dns.CANCELLED, +] // See https://nodejs.org/api/os.html#error-constants -type NodeJSErrnoErrors = typeof constants.errno -export type NodeJSErrnoErrorCodes = keyof NodeJSErrnoErrors +const errnoErrors = Object.keys(constants.errno) -const errnoErrorCodeSet = new Set(Object.keys(constants.errno)) +const errnoErrorCodeSet = new Set([errnoErrors, dnsErrorCodes].flat()) /** * NodeJS native errors with a code property. */ export type NodeJSErrnoException = NodeJS.ErrnoException & { - code: NodeJSErrnoErrorCodes - errno: number + // Unfortunately we can't make this type more concrete, as DNS error codes are not known at typescript compile time. + code: string } export type EAddrInUseException = NodeJSErrnoException & { @@ -37,7 +66,7 @@ export type EAddrInUseException = NodeJSErrnoException & { } export function isErrnoException(err: any): err is NodeJSErrnoException { - return typeof err.code === "string" && typeof err.errno === "number" && errnoErrorCodeSet.has(err.code) + return typeof err.code === "string" && errnoErrorCodeSet.has(err.code) } export function isEAddrInUseException(err: any): err is EAddrInUseException { @@ -64,7 +93,7 @@ export interface GardenErrorParams { */ readonly taskType?: string - readonly code?: NodeJSErrnoErrorCodes + readonly code?: NodeJSErrnoException["code"] } export abstract class GardenError extends Error { @@ -81,7 +110,7 @@ export abstract class GardenError extends Error { /** * If there was an underlying NodeJSErrnoException, the error code */ - public code?: NodeJSErrnoErrorCodes + public code?: NodeJSErrnoException["code"] public wrappedErrors?: GardenError[] @@ -354,7 +383,7 @@ export class InternalError extends GardenError { static wrapError(error: Error | string | any, prefix?: string): InternalError { let message: string | undefined let stack: string | undefined - let code: NodeJSErrnoErrorCodes | undefined + let code: NodeJSErrnoException["code"] | undefined if (isErrnoException(error)) { message = error.message diff --git a/core/src/plugins/kubernetes/retry.ts b/core/src/plugins/kubernetes/retry.ts index f27cd489f9..87c54d6d4e 100644 --- a/core/src/plugins/kubernetes/retry.ts +++ b/core/src/plugins/kubernetes/retry.ts @@ -15,8 +15,9 @@ import { dedent, deline } from "../../util/string" import { LogLevel } from "../../logger/logger" import { KubernetesError } from "./api" import requestErrors = require("request-promise/errors") -import { InternalError, NodeJSErrnoErrorCodes, isErrnoException } from "../../exceptions" +import { InternalError, NodeJSErrnoException, isErrnoException } from "../../exceptions" import { ErrorEvent } from "ws" +import dns from "node:dns" /** * The flag {@code forceRetry} can be used to avoid {@link shouldRetry} helper call in case if the error code @@ -93,7 +94,7 @@ export function toKubernetesError(err: unknown, context: string): KubernetesErro let response: KubernetesClientHttpError["response"] | undefined let body: any | undefined let responseStatusCode: number | undefined - let code: NodeJSErrnoErrorCodes | undefined + let code: NodeJSErrnoException["code"] | undefined if (err instanceof KubernetesClientHttpError) { errorType = "HttpError" @@ -159,7 +160,7 @@ export function toKubernetesError(err: unknown, context: string): KubernetesErro export function shouldRetry(error: unknown, context: string): boolean { const err = toKubernetesError(error, context) - if (err.code === "ECONNRESET") { + if (err.code && errorCodesForRetry.includes(err.code)) { return true } @@ -185,6 +186,8 @@ export const statusCodesForRetry: number[] = [ 524, // A Timeout Occurred ] +const errorCodesForRetry: NodeJSErrnoException["code"][] = ["ECONNRESET", dns.NOTFOUND] + const errorMessageRegexesForRetry = [ /ETIMEDOUT/, /ENOTFOUND/, diff --git a/core/test/unit/src/exceptions.ts b/core/test/unit/src/exceptions.ts index 8fa819fdde..e4963f094b 100644 --- a/core/test/unit/src/exceptions.ts +++ b/core/test/unit/src/exceptions.ts @@ -14,9 +14,54 @@ import { RuntimeError, StackTraceMetadata, getStackTraceMetadata, + isErrnoException, } from "../../../src/exceptions" import dedent from "dedent" import { testFlags } from "../../../src/util/util" +import { readFile } from "fs-extra" +import { resolve4 } from "dns/promises" +import dns from "node:dns" + +describe("isErrnoException", async () => { + it("should return true for file not found errors", async () => { + let err: unknown + + try { + await readFile("non-existent-file") + expect.fail("should have thrown") + } catch (e) { + err = e + } + + if (isErrnoException(err)) { + expect(err.code).to.equal("ENOENT") + } else { + expect.fail("should have been an NodeJSErrnoException") + } + }) + + it("should return true for DNS ENOTFOUND errors", async () => { + let err: unknown + + try { + await resolve4("non-existent-hostname") + expect.fail("should have thrown") + } catch (e) { + err = e + } + + if (isErrnoException(err)) { + expect(err.code).to.equal(dns.NOTFOUND) + } else { + expect.fail("should have been an NodeJSErrnoException") + } + }) + + it("should return false for other errors", () => { + const err = new Error("test exception") + expect(isErrnoException(err)).to.be.false + }) +}) describe("GardenError", () => { // helper to avoid dealing with changing line numbers