From a60a8b1b67b5fc3031af4de940f571279f3b64d4 Mon Sep 17 00:00:00 2001 From: Steffen Neubauer Date: Tue, 31 Oct 2023 18:28:28 +0100 Subject: [PATCH] fix: handle DNS error codes as ErrnoException DNS error codes are not included in the OS error constants (os.constants) / https://nodejs.org/docs/latest-v18.x/api/os.html#error-constants This commit adds DNS error codes to the list of handled errno errors. Fixes #5217 --- core/src/analytics/analytics.ts | 4 +-- core/src/exceptions.ts | 46 ++++++++++++++++++++++------ core/src/plugins/kubernetes/retry.ts | 12 ++++++-- 3 files changed, 48 insertions(+), 14 deletions(-) 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..1e6bf5d5ad 100644 --- a/core/src/exceptions.ts +++ b/core/src/exceptions.ts @@ -14,19 +14,47 @@ 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. +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 +65,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 +92,7 @@ export interface GardenErrorParams { */ readonly taskType?: string - readonly code?: NodeJSErrnoErrorCodes + readonly code?: NodeJSErrnoException["code"] } export abstract class GardenError extends Error { @@ -81,7 +109,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 +382,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..fe30de970f 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,11 @@ export const statusCodesForRetry: number[] = [ 524, // A Timeout Occurred ] +const errorCodesForRetry: NodeJSErrnoException["code"][] = [ + "ECONNRESET", + dns.NOTFOUND, +] + const errorMessageRegexesForRetry = [ /ETIMEDOUT/, /ENOTFOUND/,